| Message ID | 20200706034203.2171077-2-sjg@chromium.org |
|---|---|
| State | Accepted |
| Commit | 2e9a0cdfa8456636392f24dcc47e3270bd4818b7 |
| Headers | show |
| Series | RFC: patman: Collect review tags and comments from Patchwork | expand |
Hi Simon, I can't see a cover letter so apologies if I've misunderstood something basic, but this doesn't appear to apply to the patchwork tree - I'm guessing the patchwork relevance is with regards to the last few patches that (AFAICT) parse the patchwork web interface for information? I haven't fully digested the patches (and I lack a lot of context) but is there a reason the patchwork API isn't able to meet your needs here? (And if so, could we extend it rather than having you parse the frontend?) Regards, Daniel > This handles skipped tests correctly, so use it instead of the existing > code. > > Signed-off-by: Simon Glass <sjg at chromium.org> > --- > > tools/patman/main.py | 8 ++------ > tools/patman/test_util.py | 6 +++--- > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/tools/patman/main.py b/tools/patman/main.py > index 28a9a26087..03668d1bb8 100755 > --- a/tools/patman/main.py > +++ b/tools/patman/main.py > @@ -25,6 +25,7 @@ from patman import patchstream > from patman import project > from patman import settings > from patman import terminal > +from patman import test_util > from patman import test_checkpatch > > > @@ -101,12 +102,7 @@ elif options.test: > suite = doctest.DocTestSuite(module) > suite.run(result) > > - # TODO: Surely we can just 'print' result? > - print(result) > - for test, err in result.errors: > - print(err) > - for test, err in result.failures: > - print(err) > + sys.exit(test_util.ReportResult('patman', None, result)) > > # Called from git with a patch filename as argument > # Printout a list of additional CC recipients for this patch > diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py > index aac58fb72f..0827488f33 100644 > --- a/tools/patman/test_util.py > +++ b/tools/patman/test_util.py > @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult): > for test, err in result.failures: > print(err, result.failures) > if result.skipped: > - print('%d binman test%s SKIPPED:' % > - (len(result.skipped), 's' if len(result.skipped) > 1 else '')) > + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname, > + 's' if len(result.skipped) > 1 else '')) > for skip_info in result.skipped: > print('%s: %s' % (skip_info[0], skip_info[1])) > if result.errors or result.failures: > - print('binman tests FAILED') > + print('%s tests FAILED' % toolname) > return 1 > return 0 > > -- > 2.27.0.212.ge8ba1cc988-goog > > _______________________________________________ > Patchwork mailing list > Patchwork at lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
Daniel Axtens <dja at axtens.net> writes: > Hi Simon, > > I can't see a cover letter so apologies if I've misunderstood something > basic, but this doesn't appear to apply to the patchwork tree - I'm > guessing the patchwork relevance is with regards to the last few patches > that (AFAICT) parse the patchwork web interface for information? Ah, nevermind, the cover letter got caught in moderation. I've released it. pwclient speaks the old, less documented XML-RPC API. We have a new REST API which is much better documented, and is explorable (e.g. https://patchwork.ozlabs.org/api/ and https://patchwork.readthedocs.io/en/latest/api/rest/ ) > I haven't fully digested the patches (and I lack a lot of context) but > is there a reason the patchwork API isn't able to meet your needs here? > (And if so, could we extend it rather than having you parse the frontend?) So these questions still stand but for the REST API. Regards, Daniel > > Regards, > Daniel > >> This handles skipped tests correctly, so use it instead of the existing >> code. >> >> Signed-off-by: Simon Glass <sjg at chromium.org> >> --- >> >> tools/patman/main.py | 8 ++------ >> tools/patman/test_util.py | 6 +++--- >> 2 files changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/tools/patman/main.py b/tools/patman/main.py >> index 28a9a26087..03668d1bb8 100755 >> --- a/tools/patman/main.py >> +++ b/tools/patman/main.py >> @@ -25,6 +25,7 @@ from patman import patchstream >> from patman import project >> from patman import settings >> from patman import terminal >> +from patman import test_util >> from patman import test_checkpatch >> >> >> @@ -101,12 +102,7 @@ elif options.test: >> suite = doctest.DocTestSuite(module) >> suite.run(result) >> >> - # TODO: Surely we can just 'print' result? >> - print(result) >> - for test, err in result.errors: >> - print(err) >> - for test, err in result.failures: >> - print(err) >> + sys.exit(test_util.ReportResult('patman', None, result)) >> >> # Called from git with a patch filename as argument >> # Printout a list of additional CC recipients for this patch >> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py >> index aac58fb72f..0827488f33 100644 >> --- a/tools/patman/test_util.py >> +++ b/tools/patman/test_util.py >> @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult): >> for test, err in result.failures: >> print(err, result.failures) >> if result.skipped: >> - print('%d binman test%s SKIPPED:' % >> - (len(result.skipped), 's' if len(result.skipped) > 1 else '')) >> + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname, >> + 's' if len(result.skipped) > 1 else '')) >> for skip_info in result.skipped: >> print('%s: %s' % (skip_info[0], skip_info[1])) >> if result.errors or result.failures: >> - print('binman tests FAILED') >> + print('%s tests FAILED' % toolname) >> return 1 >> return 0 >> >> -- >> 2.27.0.212.ge8ba1cc988-goog >> >> _______________________________________________ >> Patchwork mailing list >> Patchwork at lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/patchwork
Hi Daniel, On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja at axtens.net> wrote: > > Daniel Axtens <dja at axtens.net> writes: > > > Hi Simon, > > > > I can't see a cover letter so apologies if I've misunderstood something > > basic, but this doesn't appear to apply to the patchwork tree - I'm > > guessing the patchwork relevance is with regards to the last few patches > > that (AFAICT) parse the patchwork web interface for information? > > Ah, nevermind, the cover letter got caught in moderation. I've released it. > > pwclient speaks the old, less documented XML-RPC API. We have a new > REST API which is much better documented, and is explorable > (e.g. https://patchwork.ozlabs.org/api/ and > https://patchwork.readthedocs.io/en/latest/api/rest/ ) > > > I haven't fully digested the patches (and I lack a lot of context) but > > is there a reason the patchwork API isn't able to meet your needs here? > > (And if so, could we extend it rather than having you parse the frontend?) > > So these questions still stand but for the REST API. So use the REST API instead of the web page? That sounds fine to me. Is it generally enabled on patchwork servers? What is the status of pwclient? Is it dead? Is there a replacement? I'd love to use a Python library if one exists. Regards, SImon
Simon Glass <sjg at chromium.org> writes: > Hi Daniel, > > On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja at axtens.net> wrote: >> >> Daniel Axtens <dja at axtens.net> writes: >> >> > Hi Simon, >> > >> > I can't see a cover letter so apologies if I've misunderstood something >> > basic, but this doesn't appear to apply to the patchwork tree - I'm >> > guessing the patchwork relevance is with regards to the last few patches >> > that (AFAICT) parse the patchwork web interface for information? >> >> Ah, nevermind, the cover letter got caught in moderation. I've released it. >> >> pwclient speaks the old, less documented XML-RPC API. We have a new >> REST API which is much better documented, and is explorable >> (e.g. https://patchwork.ozlabs.org/api/ and >> https://patchwork.readthedocs.io/en/latest/api/rest/ ) >> >> > I haven't fully digested the patches (and I lack a lot of context) but >> > is there a reason the patchwork API isn't able to meet your needs here? >> > (And if so, could we extend it rather than having you parse the frontend?) >> >> So these questions still stand but for the REST API. > > So use the REST API instead of the web page? That sounds fine to me. > Is it generally enabled on patchwork servers? I mean patman is your code so it's ultimately not my call :P But yes, I'd strongly prefer you used the REST API! It is enabled on ozlabs.org and kernel.org and has been for a while (~a couple of years). > What is the status of pwclient? Is it dead? Is there a replacement? > I'd love to use a Python library if one exists. Stephen F is the expert on the client stuff, so I'm not going to make a call on the status of pwclient. All I am confident to say is that I have migrated to using 'git-pw' and I recommend others do so to too. I'm not sure if a dedicated Python client library exists: the last time I wanted to write a Python client app I found it simple enough to just use the JSON that the API provides directly. But the place I'd start with is git-pw. Regards, Daniel > Regards, > SImon
On Tue, 2020-07-07 at 11:09 +1000, Daniel Axtens wrote: > Simon Glass <sjg at chromium.org> writes: > > > Hi Daniel, > > > > On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja at axtens.net> wrote: > > > Daniel Axtens <dja at axtens.net> writes: > > > > > > > Hi Simon, > > > > > > > > I can't see a cover letter so apologies if I've misunderstood something > > > > basic, but this doesn't appear to apply to the patchwork tree - I'm > > > > guessing the patchwork relevance is with regards to the last few patches > > > > that (AFAICT) parse the patchwork web interface for information? > > > > > > Ah, nevermind, the cover letter got caught in moderation. I've released it. > > > > > > pwclient speaks the old, less documented XML-RPC API. We have a new > > > REST API which is much better documented, and is explorable > > > (e.g. https://patchwork.ozlabs.org/api/ and > > > https://patchwork.readthedocs.io/en/latest/api/rest/ ) > > > > > > > I haven't fully digested the patches (and I lack a lot of context) but > > > > is there a reason the patchwork API isn't able to meet your needs here? > > > > (And if so, could we extend it rather than having you parse the frontend?) > > > > > > So these questions still stand but for the REST API. > > > > So use the REST API instead of the web page? That sounds fine to me. > > Is it generally enabled on patchwork servers? > > I mean patman is your code so it's ultimately not my call :P But yes, > I'd strongly prefer you used the REST API! It is enabled on ozlabs.org > and kernel.org and has been for a while (~a couple of years). > > > What is the status of pwclient? Is it dead? Is there a replacement? > > I'd love to use a Python library if one exists. > > Stephen F is the expert on the client stuff, so I'm not going to make a > call on the status of pwclient. All I am confident to say is that I have > migrated to using 'git-pw' and I recommend others do so to too. pwclient is alive and won't be going anywhere, but it's still using the now-deprecated XML-RPC API. Eventually this will move to the REST API, but I just haven't found the time to do so yet. > I'm not sure if a dedicated Python client library exists: the last time > I wanted to write a Python client app I found it simple enough to just > use the JSON that the API provides directly. But the place I'd start > with is git-pw. There's no patchwork SDK yet and git-pw is intended for use as a tool rather than a library (the internal API isn't stable). However, the Patchwork API itself is very trivial so requests should get you what you want very easily. Stephen > Regards, > Daniel > > > Regards, > > SImon
diff --git a/tools/patman/main.py b/tools/patman/main.py index 28a9a26087..03668d1bb8 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -25,6 +25,7 @@ from patman import patchstream from patman import project from patman import settings from patman import terminal +from patman import test_util from patman import test_checkpatch @@ -101,12 +102,7 @@ elif options.test: suite = doctest.DocTestSuite(module) suite.run(result) - # TODO: Surely we can just 'print' result? - print(result) - for test, err in result.errors: - print(err) - for test, err in result.failures: - print(err) + sys.exit(test_util.ReportResult('patman', None, result)) # Called from git with a patch filename as argument # Printout a list of additional CC recipients for this patch diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index aac58fb72f..0827488f33 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult): for test, err in result.failures: print(err, result.failures) if result.skipped: - print('%d binman test%s SKIPPED:' % - (len(result.skipped), 's' if len(result.skipped) > 1 else '')) + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname, + 's' if len(result.skipped) > 1 else '')) for skip_info in result.skipped: print('%s: %s' % (skip_info[0], skip_info[1])) if result.errors or result.failures: - print('binman tests FAILED') + print('%s tests FAILED' % toolname) return 1 return 0
This handles skipped tests correctly, so use it instead of the existing code. Signed-off-by: Simon Glass <sjg at chromium.org> --- tools/patman/main.py | 8 ++------ tools/patman/test_util.py | 6 +++--- 2 files changed, 5 insertions(+), 9 deletions(-)