Hi Simon, On 06. 07. 20 5:41, Simon Glass wrote:
Patman is a useful tool for creating, checking sending out patches. It automates the creation of patches, simplifies checking them and handles cover letters and change logs.
However once patches are sent and reviewers add Reviewed-by tags, etc., these must be manually added into the commits using git before the next version of the series is sent. Also, the only way to look at patches is in the web interface.
It would be nice if patman could talk to Patchwork and collect the review tags itself. Even better if it could show the review comments in a command-line view patch-by-patch to speed up addressing comments.
This series adds these features to patman, talking directly to the web URLs. While pwclient seems like it could handle some of this, or at least provide a library to patman, the documentation[1] doesn't really explain what the commands do and it doesn't seem to work with the current Patchwork (e.g. pwclient git-am requires a patch number but Patchwork seems to use an ID now).
With these changes, which currently lack tests, it is possible to type 'patman status' and see a list of new review tags. It is also possible to create a new branch with those tags and list out the review comments as small snippets on the command line.
I have been using these features for a short while and they seem useful, so herewith a series.
This series depends on u-boot-dm/next so you might find it easier to get it from u-boot-dm/patman-working[2]
[1] https://patchwork.readthedocs.io/projects/pwclient/en/latest/usage/ [2] https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/patman-working
Simon Glass (16): patman: Use test_util to show test results patman: Move main code out to a control module patman: Add a test that uses gitpython patman: Allow creating patches for another branch patman: Allow skipping patches at the end patman: Convert to ArgumentParser patman: Allow different commands patman: Add a 'test' subcommand patman: Allow disabling 'bright' mode with Print output patman: Support collecting response tags in Patchstream patman: Allow linking a series with patchwork patman: Add a -D option to enable debugging patchstream: Support parsing of review snippets patman: Support checking for review tags in patchwork patman: Support updating a branch with review tags patman: Support listing comments from patchwork
.azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- .travis.yml | 2 +- test/run | 2 +- tools/patman/README | 93 ++++++- tools/patman/checkpatch.py | 6 + tools/patman/commit.py | 14 ++ tools/patman/control.py | 214 ++++++++++++++++ tools/patman/func_test.py | 170 ++++++++++++- tools/patman/gitutil.py | 23 +- tools/patman/main.py | 219 ++++++++--------- tools/patman/patchstream.py | 82 ++++++- tools/patman/series.py | 4 +- tools/patman/settings.py | 10 +- tools/patman/status.py | 476 ++++++++++++++++++++++++++++++++++++ tools/patman/terminal.py | 4 +- tools/patman/test_util.py | 8 +- tools/patman/tools.py | 4 +- 18 files changed, 1178 insertions(+), 157 deletions(-) create mode 100644 tools/patman/control.py create mode 100644 tools/patman/status.py
I have tried your series because I use patman for quite a long time. I have done these steps:
... git pull https://gitlab.denx.de/u-boot/custodians/u-boot-dm.git patman-working git-cherry-pick <sha1> (my sifive v2 series) -- Add Series-link based on patchwork which points to v2
HEAD commit looks like this:
commit b99568c40f4a2017a5dccbf96c4e5bb5ab0c7ea3 (HEAD -> simon) Author: Michal Simek michal.simek@xilinx.com AuthorDate: Thu Jul 9 10:58:47 2020 +0200 Commit: Michal Simek michal.simek@xilinx.com CommitDate: Wed Jul 15 10:05:51 2020 +0200
serial: Fix SIFIVE debug serial dependency
The commit 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for debug consoles") has added incorrect dependency for SIFIVE debug uart which should depend on SIFIVE driver instead of PL01x.
Fixes: 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for debug consoles") Signed-off-by: Michal Simek michal.simek@xilinx.com
Series-to: uboot Series-version: 2 Series-changes: 2 - Add fixes tag - asked by Simon
Series-link: https://patchwork.ozlabs.org/project/uboot/list/?series=188865
And the running status on last one patch (because your series has also other patches. [u-boot](simon)$ rm -f 0* && /mnt/disk/u-boot/tools/patman/patman status -c 1 -D patman: ValueError: Cannot parse sequence spec 'v2'
Traceback (most recent call last): File "/mnt/disk/u-boot/tools/patman/patman", line 167, in <module> args.show_comments) File "/home/monstr/data/disk/u-boot/tools/patman/../patman/control.py", line 214, in patchwork_status show_comments) File "/home/monstr/data/disk/u-boot/tools/patman/../patman/status.py", line 433, in check_patchwork_status patches = collect_patches(series, url) File "/home/monstr/data/disk/u-boot/tools/patman/../patman/status.py", line 271, in collect_patches patch.add(name, col.get_text()) File "/home/monstr/data/disk/u-boot/tools/patman/../patman/status.py", line 135, in add raise ValueError("Cannot parse sequence spec '%s'" % seq_info) ValueError: Cannot parse sequence spec 'v2'
What do you think can be wrong? I expect your regex strings.
Anyway here are my comments. 1) -e options is not recorded in readme
2) this is the first time I read about patman test and I see that README should be also updated and remove at least --test which you switched to subcommand test Would be also good to see what that command is really doing. It means having verbose mode or so would be helpful. Anyway based on strace I see that it touches patman/test/ folder that's why it indicates that it is just patman internal tests. If yes - this patchwork feature should be also added there.
3) You are collecting responses as Fixes, Tested-by, etc. I am missing there also Reported-by:
4) Last but not least is that Series-link. It seems to me that it is quite time consuming to search for it and this would require more work to do. When patman send message to mailing list it also generate Message-id. I think you could be able to append/amend it when message is sent. Then you should use this message-id to get status back based on it. It could be enough for communication with patchwork and collect tags and create another series.
Another thing what I see is that after sending you have all message ID and you can use the same style as Linux is using for setting up Link: to lore.kernel.org/r/ With the same style you can add Link: https://patchwork.ozlabs.org/project/uboot/patch/<Message-id> when you send patches.
Then you can use this Link which will be recorded in every patch to show status/feedback/collect tags, etc.
Thanks, Michal