Re: Make Acked/Reviewd/Tested-by tags visible

2014-06-11 Thread Thomas Petazzoni
Dear Jeremy Kerr,

On Wed, 11 Jun 2014 10:00:54 +1000, Jeremy Kerr wrote:

> They should be parsed when the original patch is received, and when any
> follow-ups are appended to the patch. However:
> 
> > I noticed that (in the Buildroot patchwork [0]) some tags were not
> > accounted for:
> > http://patchwork.ozlabs.org/patch/354654/  (missing reviewed-by)
> > http://patchwork.ozlabs.org/patch/354590/  (missing acked-by)
> > [...]
> 
> .. it looks like I may be missing a case there. I'll take a look.

Another thing that is missing IMO is a way of sorting the patches by
the number of A/R/T tags. Probably not needed to be able to sort them
for each individual value, but being able to sort them by the sum of
A+R+T would be useful so that maintainers can spot immediately which
patches have received Acked-by, Tested-by or Reviewed-by tags.

Thanks for this very useful new feature!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


Re: Make Acked/Reviewd/Tested-by tags visible

2014-06-11 Thread Jeremy Kerr
Hi Thomas,

> Another thing that is missing IMO is a way of sorting the patches by
> the number of A/R/T tags. Probably not needed to be able to sort them
> for each individual value, but being able to sort them by the sum of
> A+R+T would be useful so that maintainers can spot immediately which
> patches have received Acked-by, Tested-by or Reviewed-by tags.

The part I'm working on at the moment is a facility to filter the patch
list by the presence of each tag type (eg, only show patches with at
least one ack).

Would that suit? Or do you actually want to prioritise patches by the
number of acks/reviews/tests? Or rather than a boolean "presence of a
tag" filter, how about a "has N or more" type filter?

Cheers,


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


Re: Make Acked/Reviewd/Tested-by tags visible

2014-06-11 Thread Thomas Petazzoni
Dear Jeremy Kerr,

On Wed, 11 Jun 2014 17:18:19 +1000, Jeremy Kerr wrote:

> > Another thing that is missing IMO is a way of sorting the patches by
> > the number of A/R/T tags. Probably not needed to be able to sort them
> > for each individual value, but being able to sort them by the sum of
> > A+R+T would be useful so that maintainers can spot immediately which
> > patches have received Acked-by, Tested-by or Reviewed-by tags.
> 
> The part I'm working on at the moment is a facility to filter the patch
> list by the presence of each tag type (eg, only show patches with at
> least one ack).
> 
> Would that suit? Or do you actually want to prioritise patches by the
> number of acks/reviews/tests? Or rather than a boolean "presence of a
> tag" filter, how about a "has N or more" type filter?

All I was thinking of is really simple: adding the same small greenish
arrow that you use for Date sorting to do A/R/T sorting.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


[PATCH] Fallback to common charsets when charset is None or x-unknown

2014-06-11 Thread Siddhesh Poyarekar
Trying again after signing up to the mailing list (patch is slightly
modified from my first submission, which may either be in moderation
or may have gotten lost somehow):

On Wed, Jun 11, 2014 at 04:09:16PM +0530, Siddhesh Poyarekar wrote:
> Hi,
> 
> We recently encountered a case in our glibc patchwork instance on
> sourceware, where a patch was dropped because it had x-unknown
> charset.  I used the following patch to fix this in our instance.  The
> fix I used was to fall back on a set of encodings (instead of just
> utf-8) when the charset is not mentioned or if it is set as x-unknown.
> 
> I hope this is useful.  I'd love to know if you all think there is a
> better way to fix this so that I can implement that in our instance
> instead of my hack.
> 
> Cheers,
> Siddhesh

--- a/apps/patchwork/bin/parsemail.py   2014-06-11 15:53:12.685666812 +0530
+++ b/apps/patchwork/bin/parsemail.py   2014-06-11 15:53:03.991667186 +0530
@@ -147,6 +147,13 @@
 return match.group(1)
 return None
 
+def try_decode(payload, charset):
+try:
+payload = unicode(payload, charset)
+except UnicodeDecodeError:
+return None
+return payload
+
 def find_content(project, mail):
 patchbuf = None
 commentbuf = ''
@@ -157,15 +164,27 @@
 continue
 
 payload = part.get_payload(decode=True)
-charset = part.get_content_charset()
 subtype = part.get_content_subtype()
 
-# if we don't have a charset, assume utf-8
-if charset is None:
-charset = 'utf-8'
-
 if not isinstance(payload, unicode):
-payload = unicode(payload, charset)
+charset = part.get_content_charset()
+
+# If there is no charset or if it is unknown, then try some common
+# charsets before we fail.
+if charset is None or charset == 'x-unknown':
+try_charsets = ['utf-8', 'windows-1252', 'ascii', 'iso-8859-1']
+else:
+try_charsets = [charset]
+
+for cset in try_charsets:
+decoded_payload = try_decode(payload, cset)
+if decoded_payload is not None:
+break
+payload = decoded_payload
+
+# Could not find a valid decoded payload.  Fail.
+if payload is None:
+return (None, None)
 
 if subtype in ['x-patch', 'x-diff']:
 patchbuf = payload


pgp4chE7S_4UN.pgp
Description: PGP signature
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork


Re: Make Acked/Reviewd/Tested-by tags visible

2014-06-11 Thread Keller, Jacob E
On Wed, 2014-06-11 at 17:18 +1000, Jeremy Kerr wrote:
> Hi Thomas,
> 
> > Another thing that is missing IMO is a way of sorting the patches by
> > the number of A/R/T tags. Probably not needed to be able to sort them
> > for each individual value, but being able to sort them by the sum of
> > A+R+T would be useful so that maintainers can spot immediately which
> > patches have received Acked-by, Tested-by or Reviewed-by tags.
> 
> The part I'm working on at the moment is a facility to filter the patch
> list by the presence of each tag type (eg, only show patches with at
> least one ack).
> 
> Would that suit? Or do you actually want to prioritise patches by the
> number of acks/reviews/tests? Or rather than a boolean "presence of a
> tag" filter, how about a "has N or more" type filter?
> 

I wouldn't mind the filters to check for presence of tag type, but if
the sort by count of total tags would be easier, that is fine too.
Either one would be useful.

Thanks,
Jake

> Cheers,
> 
> 
> Jeremy
> ___
> 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


[PATCH] post-receive: exclude commits from the patch update step

2014-06-11 Thread Brian Norris
When merging upstream work related to other projects into your own
project repository, you probably don't want to check for (and try to
update) the status on every change-set in the merge. So add a list of
references (branches, tags, commits, etc.) whose commits should be
ignored in the patch update step.

This could be used, for example, to set:

EXCLUDE="refs/heads/upstream"

Then when you're ready to merge in new upstream code, you first update
the 'upstream' branch before pushing your own.

Signed-off-by: Brian Norris 
---
 tools/post-receive.hook | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/post-receive.hook b/tools/post-receive.hook
index 4fb741d3ea98..a38522e22f35 100755
--- a/tools/post-receive.hook
+++ b/tools/post-receive.hook
@@ -8,6 +8,12 @@ set -eu
 
 #TODO: the state map should really live in the repo's git-config
 STATE_MAP="refs/heads/master:Accepted"
+#
+# ignore all commits already present in these refs
+# e.g.,
+#   EXCLUDE="refs/heads/upstream refs/heads/other-project"
+#
+EXCLUDE=""
 
 PWDIR=/srv/patchwork/apps/patchwork
 
@@ -39,7 +45,8 @@ set_patch_state()
 update_patches()
 {
   local cnt; cnt=0
-  for rev in $(git rev-list --no-merges --reverse ${1}..${2}); do
+  for rev in $(git rev-parse --not ${EXCLUDE} |
+   git rev-list --stdin --no-merges --reverse ${1}..${2}); do
 if [ "$do_exit" = 1 ]; then
   echo "I: exiting..." >&2
   break
-- 
1.9.1

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