[PATCH 2/5] patch-list: refactored html and patch list forms

Jonathan Nieder jrnieder at gmail.com
Tue Jul 20 11:47:33 AEST 2021


Hi,

Raxel Gutierrez wrote:

> Refactored the patch list frontend code to make the forms more healthy
> and readable. Also, separated script tags into a separate js file which
> is better for keeping things modular and maintainable.

Tiny nit: patchwork's commit messages tend to use the imperative mood,
as though ordering the codebase to change.  In other words, you can
think of a commit message as kind of like a bug report --- it
describes a change you would like the project to make and why.

In this example, I think the idea is something like

	Clean up the patch-list page by <explanation>.  This allows <description
	of benefit>.  No user-visible change intended.

	Also move patch list related js code to a new patch-list.js file, to
	make the javascript easy to read and edit in one place.  This makes
	automatic code formatting easier, makes it more straightforward to
	measure test coverage and discover opportunities for refactoring, and
	simplifies a possible future migration to typescript if the project
	chooses to go in that direction.

>                                                        Refer to cover
> letter for other conventions that I suggest for frontend code like
> naming for HTML attributes / CSS selectors.

The commit message, unlike the cover letter, becomes part of the
history I can see in the patchwork repository with "git log" (e.g.,
when trying to understand a line I am investigating using "git
blame"); as a result, it's helpful for the commit messages to be
mostly self-contained.  Are there particular conventions that are
relevant for this patch that should be included here?  Or does this
suggest that it might make sense to add a file about frontend coding
style to docs/development/?

Thanks and hope that helps,
Jonathan


More information about the Patchwork mailing list