State diagram of patch status, possible bug in apps/patchwork/utils.py

[Jeremy Kerr]

Hi,

> 1. I could not find any documentation for patchwork, neither online
> nor in the downloaded source code.

There's a little documentation in the doc/ directory, but not a whole 
lot. I'd like to add some online help to patchwork though.

> 2. What's the expected or recomended patch lifecycle status diagram?
> Will "Under review" automatically appear once the first Acked-by
> comment happens? Having to extract logic from the application code is
> less than ideal.

There's no expected flow; it's really up to the maintainers of the 
project. It's also why the patch states are configurable.

However, this is generally what we do for powerpc:

When the maintainer (benh) is looking for patches to merge, he scans the 
patchwork list for patches in one of the 'Action Required' states (ie, 
New and Under Review). For each of these, they are either:

 * Added to a 'merge' bundle;
 * Allocated to a sub-maintainer; or
 * Updated to a Rejected/Superseded/Not Applicable/etc state

Then the merge bundle is downloaded, and applied with a custom script, 
which updates the patchwork states accordingly (using pwclient).

> 3. apps/patchwork/models.py line 291 looks for the following regexp:
> '^([Tt]ested|[Rr]eviewed|[Aa]cked|[Ss]igned-off|[Nn]acked)-by: .*$'
> That regexp won't catch oddbal capitalizations like Signed-Off-by
> unless I'm mistaken.

I've just pushed a change for this. Had it sitting in my local tree for 
a while :/

> 4. apps/patchwork/templatetags/syntax.py line 41ff is missing
> Nacked-by.

Fixed and pushed.

> 5. How can I automatically match commits against pending patches? We
> use one centralized subversion repository and the mailing list gets a
> commit mail for every commit. If matching committed patches
> automatically against pending patches is not possible, can we have
> patchwork parse replies to patches which usually say something like
> "...comitted in r1234" or "thanks, r1234"?

You can use the patch 'hash' for this. It's a bit hacky for now, but 
still possible:

$ python ./apps/patchwork/parse.py --hash < foo.patch
7951ad5e4f12f93adaced1718f713f6a70979dcd

This hash is based on the content of the patch, minus stuff that's 
likely to change (eg line numbers). You can then use this hash to match 
a patch in patchwork:

$ pwclient update -s Accepted -c <commit-ref> -h <hash>

> 6. Some parts of patchwork seem to hardcode git usage. Is that just
> because nobody had the time to add subversion support as well?

Which bits? patchwork should be VCS-neutral - if there's anything that's 
too specific to git, then I can look at making it work with svn too.

> 7. Possible bug in the unarchive action in apps/patchwork/utils.py,
> patch follows:
>
> diff --git a/apps/patchwork/utils.py b/apps/patchwork/utils.py
> index 797951c..fa26aef 100644
> --- a/apps/patchwork/utils.py
> +++ b/apps/patchwork/utils.py
> @@ -183,7 +183,7 @@ def set_patches(user, project, action, data,
> patches, context): str = 'archived'
>
>          elif action == 'unarchive':
> -            patch.archived = True
> +            patch.archived = False
>              patch.save()
>              str = 'un-archived'

Thanks, applied.

Cheers,


Jeremy