[PATCH 4/5] static: utils.js for modular handling of requests & messages

Jonathan Nieder jrnieder at gmail.com
Tue Jul 20 12:13:14 AEST 2021


Hi,

Raxel Gutierrez wrote:

> Added file to have a general utilities JavaScript module file for
> functions that are reused throughout various Patchwork js files. First
> added function for making fetch requests to update properties through
> 'PATCH' requests with the REST api endpoints. Also, added functions for
> handling update & error messages for these requests. The subsequent
> patch will make use of these functions which will be also reused in
> future features the make use fetch requests to update object fields.
>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
>  htdocs/README.rst  |  7 +++++
>  htdocs/js/utils.js | 71 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
>  create mode 100644 htdocs/js/utils.js

I think I'd prefer to see this squashed into the same patch as patch
5/5, because that way it's easier to evaluate the interface it exposes
in the context of what its callers need.  I don't feel strongly about
that, though.  (It mostly comes down to whether the API exposed here
is obvious enough when viewed without that context.)

> diff --git a/htdocs/README.rst b/htdocs/README.rst
> index 4441bf3..2bae34c 100644
> --- a/htdocs/README.rst
> +++ b/htdocs/README.rst
> @@ -147,3 +147,10 @@ js
>    :Website: https://selectize.github.io/selectize.js/
>    :GitHub: https://github.com/selectize/selectize.js
>    :Version: 0.11.2
> +
> +``utils.js.``
> +
> +  General utility module for functions used throughout other static Patchwork
> +  js files (fetch requests, handling update & error messages).
> +
> +  Part of Patchwork.

These appear to be specifically about making REST API requests to the
patchwork backend, so how about a name like rest.js?

That way, if we come up with other utility functions then we can group
them into their own utility library.

Thanks,
Jonathan


More information about the Patchwork mailing list