Re: [PATCH] Find patches, cover letters and comments by msgid

2018-08-26 Thread Linus Torvalds
On Sun, Aug 26, 2018 at 7:14 AM Daniel Axtens  wrote:
>
> This patch doesn't expose these URLs in the UI anywhere. I'd love to
> know where people would like them to go. I hesitate to make the
> message-id clickable in the patch detail view, as I think that would
> break workflows for people who want to copy the msgid to the clipboard.

So just to start off: I'm not personally a heavy patchwork user, but I
end up occasionally using it particularly when there's a bug report
and the issue bisects down to something that then has a patchwork
link.

I've also mostly seen old versions of patchwork becasue of kernel.org
not having been up-to-date.

Take this to mean that my opinions on this may be skewed and not
necessarily those of heavy users.

But given that, I'd really like to see a pointer to the actual email
in the header line not just of the original email with the patch, but
of every comment. Right now patchwork (well, at least on kernel.org)
has that separate header area for that particular patchwork entry, I
don't mean that. I mean the line that right now has a pointer to the
author of each comment, and a date.

Add a clickable link to just after the name of the person making the
comment? Or even make the existing (mostly useless) date clickable?

Also, since the first email is likely the one you care about most,
also make the "Message ID" string itself clickable (in _addition_ to
the one-line header for the "Commit message" like all the "Comments").

I agree that the actual string that contains the message ID itself
might be best not be clickable for the whole copy-paste reason you
mention, but the actual "Message ID" string itself could be clickable,
no?

IOW (sorry for the ugly ASCII barfic), in the header of the whole
thing, you'd have:

  Message ID  20180826141413.3033-1-...@axtens.net
  ^^  
  ||
  |+--- not clickable to make
  | copy-paste easy
  |
  +-- clickable, to get to the email

and then in the header for the individual "Commit message" and
"Comments", you'd have

  Daniel Axtens   [ msg ]  Mon, 27 Aug 2018 00:14:13
  ^   ^^^
  | |
  | +--- clickable, goes to the message
  |
  +-- clickable, goes to the person

Or _something_ along these lines. Maybe instead of that added "[msg]"
thing, just make the date clickable and get to the email instead.

   Linus
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


[PATCH] Find patches, cover letters and comments by msgid

2018-08-26 Thread Daniel Axtens
Add /message/ and /project//message/ URLs.

/project//message/ finds the patch, cover-letter or
comment in  that matches  and redirects you to it.
This is guaranteed to be unique.

/message/ does not require a project, but therefore if a mail
has been sent to multiple projects, the project that you will be
redirected to is arbitrary.

This patch doesn't expose these URLs in the UI anywhere. I'd love to
know where people would like them to go. I hesitate to make the
message-id clickable in the patch detail view, as I think that would
break workflows for people who want to copy the msgid to the clipboard.

These URLs also just redirect to the usual canonical patchwork URL -
it doesn't supplant or replace them. However, I'd be open to moving to
this scheme as the 'normal'/default URL in the future.

I also haven't attempted to do anything meaningful with series.

Partially-closes: #106
Reported-by: Konstantin Ryabitsev 
Reported-by: Linus Torvalds 
Reported-by: Stephen Finucane 
Signed-off-by: Daniel Axtens 
---
 patchwork/urls.py|  7 ++
 patchwork/views/patch.py | 49 
 2 files changed, 56 insertions(+)

diff --git a/patchwork/urls.py b/patchwork/urls.py
index 6b1ef511ef15..90391b90e413 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -74,6 +74,13 @@ urlpatterns = [
 url(r'^series/(?P\d+)/mbox/$', series_views.series_mbox,
 name='series-mbox'),
 
+# message-id searches
+url(r'^project/(?P[^/]+)/message/(?P[^/]+)$',
+patch_views.patch_by_project_msgid,
+name='msgid-project-redirect'),
+url(r'^message/(?P[^/]+)$', patch_views.patch_by_msgid,
+name='msgid-redirect'),
+
 # logged-in user stuff
 url(r'^user/$', user_views.profile, name='user-profile'),
 url(r'^user/todo/$', user_views.todo_lists,
diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 6921882e71d1..53441b39cc57 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -28,6 +28,7 @@ from django.urls import reverse
 
 from patchwork.forms import CreateBundleForm
 from patchwork.forms import PatchForm
+from patchwork.models import Comment
 from patchwork.models import Bundle
 from patchwork.models import Patch
 from patchwork.models import Project
@@ -150,3 +151,51 @@ def patch_mbox(request, patch_id):
 patch.filename)
 
 return response
+
+
+def patch_by_msgid(request, msg_id):
+msgid = ('<%s>' % msg_id)
+
+patches = Patch.objects.filter(msgid=msgid)
+if patches:
+return HttpResponseRedirect(
+reverse('patch-detail', kwargs={'patch_id': 
patches.first().id}))
+
+subs = Submission.objects.filter(msgid=msgid)
+if subs:
+return HttpResponseRedirect(
+reverse('cover-detail', kwargs={'cover_id': subs.first().id}))
+
+comments = Comment.objects.filter(msgid=msgid)
+if comments:
+return HttpResponseRedirect(comments.first().get_absolute_url())
+
+raise Http404("No patch, cover letter of comment matching %s found." % 
msg_id)
+
+
+def patch_by_project_msgid(request, project_id, msg_id):
+project = get_object_or_404(Project, linkname=project_id)
+project_id = project.id
+msgid = ('<%s>' % msg_id)
+
+try:
+patch = Patch.objects.get(patch_project_id=project_id, msgid=msgid)
+return HttpResponseRedirect(
+reverse('patch-detail', kwargs={'patch_id': patch.id}))
+except Patch.DoesNotExist:
+pass
+
+try:
+sub = Submission.objects.get(project_id=project_id, msgid=msgid)
+return HttpResponseRedirect(
+reverse('cover-detail', kwargs={'cover_id': sub.id}))
+except Submission.DoesNotExist:
+pass
+
+try:
+comment = Comment.objects.get(submission__project_id=project_id, 
msgid=msgid)
+return HttpResponseRedirect(comment.get_absolute_url())
+except Comment.DoesNotExist:
+pass
+
+raise Http404("No patch, cover letter or comment matching %s found in %s" 
% (msg_id, project.name))
-- 
2.17.1

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


Re: [PATCH] docs: Fix documentation of REST_RESULTS_PER_PAGE setting

2018-08-26 Thread Daniel Axtens
Andrew Donnellan  writes:

> In 8fe11180a1a5 ("REST: Add new setting for maximum API page size") I
> accidentally deleted the versionadded information for
> REST_RESULTS_PER_PAGE. Restore it.
>
> Fixes: 8fe11180a1a5 ("REST: Add new setting for maximum API page size")
> Signed-off-by: Andrew Donnellan 

Thanks Andrew - applied.

Regards,
Daniel
> ---
>  docs/deployment/configuration.rst | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/docs/deployment/configuration.rst 
> b/docs/deployment/configuration.rst
> index e599522a412b..0601276a3cbc 100644
> --- a/docs/deployment/configuration.rst
> +++ b/docs/deployment/configuration.rst
> @@ -88,6 +88,8 @@ Enable the :doc:`REST API <../api/rest>`.
>  The number of items to include in REST API responses by default. This can be
>  overridden by the ``per_page`` parameter for some endpoints.
>  
> +.. versionadded:: 2.0
> +
>  ``MAX_REST_RESULTS_PER_PAGE``
>  ~
>  
> -- 
> 2.11.0
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


[PATCH] docs: Fix documentation of REST_RESULTS_PER_PAGE setting

2018-08-26 Thread Andrew Donnellan
In 8fe11180a1a5 ("REST: Add new setting for maximum API page size") I
accidentally deleted the versionadded information for
REST_RESULTS_PER_PAGE. Restore it.

Fixes: 8fe11180a1a5 ("REST: Add new setting for maximum API page size")
Signed-off-by: Andrew Donnellan 
---
 docs/deployment/configuration.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/deployment/configuration.rst 
b/docs/deployment/configuration.rst
index e599522a412b..0601276a3cbc 100644
--- a/docs/deployment/configuration.rst
+++ b/docs/deployment/configuration.rst
@@ -88,6 +88,8 @@ Enable the :doc:`REST API <../api/rest>`.
 The number of items to include in REST API responses by default. This can be
 overridden by the ``per_page`` parameter for some endpoints.
 
+.. versionadded:: 2.0
+
 ``MAX_REST_RESULTS_PER_PAGE``
 ~
 
-- 
2.11.0

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


Re: [PATCH 1/1] pwclient: Fix pwclient am output formatting

2018-08-26 Thread Daniel Axtens
Hi Petr,

Applied, thank you!

Regards,
Daniel

> repr() print unicode prefix for string:
> $ pwclient git-am N
> Applying patch #N using u'git am'
>
> Remove it:
> $ pwclient git-am N
> Applying patch #918868 using "git am"
>
> git mixes single and double quotes, use double quotes which are more
> frequently used.
>
> Signed-off-by: Petr Vorel 
> ---
>  patchwork/bin/pwclient | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/patchwork/bin/pwclient b/patchwork/bin/pwclient
> index 79137b0..2020a8c 100755
> --- a/patchwork/bin/pwclient
> +++ b/patchwork/bin/pwclient
> @@ -327,8 +327,8 @@ def action_apply(rpc, patch_id, apply_cmd=None):
>  print('Applying patch #%d to current directory' % patch_id)
>  apply_cmd = ['patch', '-p1']
>  else:
> -print('Applying patch #%d using %s' %
> -  (patch_id, repr(' '.join(apply_cmd
> +print('Applying patch #%d using "%s"' %
> +  (patch_id, ' '.join(apply_cmd)))
>  
>  print('Description: %s' % patch['name'])
>  s = rpc.patch_get_mbox(patch_id)
> -- 
> 2.18.0
>
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


Re: [RFC PATCH] REST: Add new setting for maximum API page size

2018-08-26 Thread Daniel Axtens
Andrew Donnellan  writes:

> On 07/08/18 17:20, Daniel Axtens wrote:
>> I wonder if we could let authenticated users do slower queries. Maybe
>> something for later. Let's split the difference at 250 then, I'd be
>> happy to merge that.
>
> If you want to take this patch and s/500/250/g during merge then I'd be 
> happy with that.

Applied based on my trusting that OzLabs testing has been sufficent.

Regards,
Daniel

>
> -- 
> Andrew Donnellan  OzLabs, ADL Canberra
> andrew.donnel...@au1.ibm.com  IBM Australia Limited
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


Re: [PATCH 01/11] REST: Check.user is not read-only

2018-08-26 Thread Daniel Axtens
Hi Stephen,

Super keen to have Django 2.0 support! I dropped patch 8 because I
couldn't get it to work, and applied the small changes I suggested.

I have applied and pushed the remainder.

Regards,
Daniel

> We only support 'Check' creation - not check updating. As a result,
> there's no real reason that the 'Check.user' field should be read-only
> and this is causing an issue with Django REST Framework 3.7. Simply
> remove the attribute and extend the tests to validate things are working
> as expected.
>
> Signed-off-by: Stephen Finucane 
> ---
>  patchwork/api/check.py| 2 +-
>  patchwork/tests/api/test_check.py | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/patchwork/api/check.py b/patchwork/api/check.py
> index 8753c7de..5e461de6 100644
> --- a/patchwork/api/check.py
> +++ b/patchwork/api/check.py
> @@ -44,7 +44,7 @@ class CheckSerializer(HyperlinkedModelSerializer):
>  
>  url = CheckHyperlinkedIdentityField('api-check-detail')
>  patch = HiddenField(default=CurrentPatchDefault())
> -user = UserSerializer(read_only=True, default=CurrentUserDefault())
> +user = UserSerializer(default=CurrentUserDefault())
>  
>  def run_validation(self, data):
>  for val, label in Check.STATE_CHOICES:
> diff --git a/patchwork/tests/api/test_check.py 
> b/patchwork/tests/api/test_check.py
> index 43181af3..0e7e0cfc 100644
> --- a/patchwork/tests/api/test_check.py
> +++ b/patchwork/tests/api/test_check.py
> @@ -67,6 +67,7 @@ class TestCheckAPI(APITestCase):
>  self.assertEqual(check_obj.target_url, check_json['target_url'])
>  self.assertEqual(check_obj.context, check_json['context'])
>  self.assertEqual(check_obj.description, check_json['description'])
> +self.assertEqual(check_obj.user.id, check_json['user']['id'])
>  
>  def test_list(self):
>  """Validate we can list checks on a patch."""
> -- 
> 2.17.1
>
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork