Re: Make Acked/Reviewd/Tested-by tags visible
Yann == Yann E MORIN yann.morin.1...@free.fr writes: Jeremy, All, On 2014-06-03 22:15 +0800, Jeremy Kerr spake thusly: I've just rolled out the initial code to patchwork.ozlabs.org. The tag counts are all at zero to start with; I've triggered a re-parse of all of the existing comments, so the counts should reflect reality when that process finishes. On second thoughts, I've disabled the display of the counts until the update is complete, so we don't have false info in the UI in the meantime. I'll re-enable once that's done. For those not checking their web UI: the A/R/T status is now visible in the web UI! Great, thanks all! -- Bye, Peter Korsgaard ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Hi Jeremy, On Wed, Jun 11, 2014 at 2:00 AM, Jeremy Kerr j...@ozlabs.org wrote: Hi Yann, How and when are the tags parsed? 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. Here is also a problem where the tags already present in the submitted patch are not taken into account: http://patchwork.ozlabs.org/patch/359733/ http://patchwork.ozlabs.org/patch/359743/ (in fact all the patches in this series) Best regards, Thomas ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
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
Re: Make Acked/Reviewd/Tested-by tags visible
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
Re: Make Acked/Reviewd/Tested-by tags visible
Hi Yann, How and when are the tags parsed? 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. Jeremy ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Jeremy, All, On 2014-06-03 22:11 +0800, Jeremy Kerr spake thusly: I've just rolled out the initial code to patchwork.ozlabs.org. The tag counts are all at zero to start with; How and when are the tags parsed? 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) [...] [0] http://patchwork.ozlabs.org/project/buildroot/list/ Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Jeremy, All, On 2014-06-03 22:15 +0800, Jeremy Kerr spake thusly: I've just rolled out the initial code to patchwork.ozlabs.org. The tag counts are all at zero to start with; I've triggered a re-parse of all of the existing comments, so the counts should reflect reality when that process finishes. On second thoughts, I've disabled the display of the counts until the update is complete, so we don't have false info in the UI in the meantime. I'll re-enable once that's done. For those not checking their web UI: the A/R/T status is now visible in the web UI! Thanks Jeremy! Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Hi Yann, How's this look? http://ozlabs.org/~jk/tmp/patchwork-ART.png Just a little reminder on this... When are you planning on deploying that? Will it also show up in the output of pwclient? I've just rolled out the initial code to patchwork.ozlabs.org. The tag counts are all at zero to start with; I've triggered a re-parse of all of the existing comments, so the counts should reflect reality when that process finishes. The filter UI and pwclient interface are still yet to be done though; they're next on my todo list :) Cheers, Jeremy ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Hi all, I've just rolled out the initial code to patchwork.ozlabs.org. The tag counts are all at zero to start with; I've triggered a re-parse of all of the existing comments, so the counts should reflect reality when that process finishes. On second thoughts, I've disabled the display of the counts until the update is complete, so we don't have false info in the UI in the meantime. I'll re-enable once that's done. Cheers, Jeremy ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Jeremy, All, On 2014-05-07 16:42 +0800, Jeremy Kerr spake thusly: We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): OK, I have some progress on this one; but wanted to make sure what I'm doing is in the direction that'd be generally useful for you. How's this look? http://ozlabs.org/~jk/tmp/patchwork-ART.png Just a little reminder on this... When are you planning on deploying that? Will it also show up in the output of pwclient? Thanks for your good work! :-) Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
How's this look? http://ozlabs.org/~jk/tmp/patchwork-ART.png Looks great, I like it! Also, are you wanting to filter by the presence of these tags? Just non-zero? How about sorting? I would love sorting per flag. However, if filtering non-zero is much less work, I can make a workflow out of that, too. Anyone have objections to adding a small column to the default patch lists? Does anyone want to *not* have this information present? Also as a user of lists I don't maintain, I'd like to see this information. It gives some indictations when hunting down patches. Thanks a lot! Wolfram signature.asc Description: Digital signature ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Jeremy, All, On 2014-05-07 16:42 +0800, Jeremy Kerr spake thusly: We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): OK, I have some progress on this one; but wanted to make sure what I'm doing is in the direction that'd be generally useful for you. How's this look? http://ozlabs.org/~jk/tmp/patchwork-ART.png Yes, that's good! I believe you will also add this in the pwclient output, right? - there are title attributes on the column header, so you get a tooltip saying Acked-by / Reviewed-by / Tested-by when hovering over it. The actual numbers have specific tooltips too (eg, 2 Acked-by). Also, are you wanting to filter by the presence of these tags? Just non-zero? How about sorting? Filtering would be a plus, sure. I'd like to have those filters: - has a non-zero A/R/T sum - as a maintainer, I want to handle patches that have already been acked/reviewed/tested - has a zero A/R/T sum - as a contributor, I want to review patches that have not already been acked/reviewed/tested Maybe something like: pwclient list [-t|--tag N] where N is the A/R/T sum you want to filter on. Anyone have objections to adding a small column to the default patch lists? Does anyone want to *not* have this information present? In Buildroot and crosstool-NG, we do use A/R/T. Other projects (eg. VLC) does not use those flags, so it should be possible to enable/disable those flags, and disabled by default (to be backward compatible with existing management scripts.) In pwclient, maybe show the A/R/T flags only if filtering on them was requested. Thanks for putting some efforts in that! :-) Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
On Wed, May 7, 2014 at 1:42 AM, Jeremy Kerr j...@ozlabs.org wrote: Hi all, We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): OK, I have some progress on this one; but wanted to make sure what I'm doing is in the direction that'd be generally useful for you. How's this look? http://ozlabs.org/~jk/tmp/patchwork-ART.png Looks great! - there are title attributes on the column header, so you get a tooltip saying Acked-by / Reviewed-by / Tested-by when hovering over it. The actual numbers have specific tooltips too (eg, 2 Acked-by). Also, are you wanting to filter by the presence of these tags? Just non-zero? How about sorting? It would be beneficial but not crucial. Anyone have objections to adding a small column to the default patch lists? Does anyone want to *not* have this information present? default would be great! -- Cheers, Jeff ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
On Tue, Dec 03, 2013 at 11:49:27PM +0100, Yann E. MORIN wrote: Jeremy, All, On 2013-11-12 08:37 +0800, Jeremy Kerr spake thusly: We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): ID Tags State Subject 123456 ART New Wonderfull patch to apply quickly I've been looking to implement this for a while, but didn't have any good ideas about how to present this in the UI. I love your proposal here with the flags, it's simple and I think would work well for the majority of cases. Just checking: any progress on this? ;-) I'd also like to express my interest in this feature :) signature.asc Description: Digital signature ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Jeremy, All, On 2013-11-12 08:37 +0800, Jeremy Kerr spake thusly: We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): ID Tags State Subject 123456 ART New Wonderfull patch to apply quickly I've been looking to implement this for a while, but didn't have any good ideas about how to present this in the UI. I love your proposal here with the flags, it's simple and I think would work well for the majority of cases. Just checking: any progress on this? ;-) Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
On Wed, 2013-11-20 at 09:43 +0100, Wolfram Sang wrote: I think the ART flags in the patch list should never be used blindly anyway. Instead, it's an indicator of which patches are interesting to look at in more detail. Right? +1 +1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Arnout, All, On 2013-11-12 09:10 +0100, Arnout Vandecappelle spake thusly: I think the ART flags in the patch list should never be used blindly anyway. Instead, it's an indicator of which patches are interesting to look at in more detail. Right? That's the way I would use it, yes: - as a maintainer, I'd first look at patches with ART flags to apply them; - as a contributor, I'd first look at patches without ART flags to review them. Regards, Yann E. MORIN. -- .-..--.. | Yann E. MORIN | Real-Time Embedded | /\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `.---: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL| v conspiracy. | '--^---^--^' ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Make Acked/Reviewd/Tested-by tags visible
Dear Jeremy Kerr, On Tue, 12 Nov 2013 08:37:26 +0800, Jeremy Kerr wrote: We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): ID Tags State Subject 123456 ART New Wonderfull patch to apply quickly I've been looking to implement this for a while, but didn't have any good ideas about how to present this in the UI. I love your proposal here with the flags, it's simple and I think would work well for the majority of cases. Since patchwork already inserts those tags to the commit log (eg. pwclient view PATCH-ID), I hope it would be easy enough to also display it in the patch list. It's easy enough to pull these out of the comment at patch-display time, but a little more tricky with list views (without some horrible SQL joining). I'll work out a nice way to to this; I think it'll involve applying these flags when the follow-up comment is parsed. Other projects have different policies for their tagging (some require 3 acks, for example), which will introduce a little complexity there too, but that's not insurmountable either. Another thing that comes to mind is that not all Acked-by or Tested-by are equal, I believe. An Acked-by from a well-known long-time contributor will not be seen with the same level of trust that an Acked-by from an unknown newcomer. Do we want to add a mechanism of rating? But that would mean the project maintainer would have to rate developers, which is not nice. Or maybe just a list of e-mails that the project's maintainer considers as trustworthy, and only those ones would be taken into account when displaying the ART flags? Or maybe it doesn't matter too much, since in the vast majority of cases, only well-known long-time contributors are doing reviews/acks. 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
Hi Yann, We would like to suggest that the web GUI and the pwclient CLI both display such tags besides each patch, a bit like (hypotetical output of pwclient): ID Tags State Subject 123456 ART New Wonderfull patch to apply quickly I've been looking to implement this for a while, but didn't have any good ideas about how to present this in the UI. I love your proposal here with the flags, it's simple and I think would work well for the majority of cases. Since patchwork already inserts those tags to the commit log (eg. pwclient view PATCH-ID), I hope it would be easy enough to also display it in the patch list. It's easy enough to pull these out of the comment at patch-display time, but a little more tricky with list views (without some horrible SQL joining). I'll work out a nice way to to this; I think it'll involve applying these flags when the follow-up comment is parsed. Other projects have different policies for their tagging (some require 3 acks, for example), which will introduce a little complexity there too, but that's not insurmountable either. Cheers, Jeremy ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
