So I had a quick look at the implemenation of the codereview of Python which
is running on django. They did a interesting work by merging user account
management between roundup and rietveld [1].
I think it will be better to use another tool than Rietveld because since the decision of Python to stop using it and migrate to GitHub, the development has staled.
There are not so much alternative software that works with Mercurial and as pre-commit (see: https://en.wikipedia.org/wiki/List_of_tools_for_code_review).
RhodeCode: Provide full hosting and work with PR. This would be a big workflow change.
Phabricator: Also do more than codereview, written in PHP which will require new knowledge for hosting.
Patchwork: Based on the interesting idea of mailing list. But without the "send reply to ML" it will be not very easy to use for new comers.
Review Board: Describe itself as alternative to Rietveld. It also has bot: https://github.com/reviewboard/ReviewBot
I must say that my preference is going to Review Board.
I propose to setup an instance on our host to test it.
Cédric Krieradded 1 deleted label and removed 1 deleted label
Having used review board (with git and rbtools) I believe it is quite straight forward to use (both in submitting/updating patches and in reviewing them).
I do not think any more that reviewboard is a good replacement choice for some reason:
- another application to register and learn
- need customization to support nested patch
- UI more complex than Rietveld
- kind of overlap with the bug-tracker on some aspect.
I would be in favor of mailing-list (with maybe Patchwork).
Indeed using patchbomb (https://www.mercurial-scm.org/wiki/PatchbombExtension) is not so complicated.
I think review by email is very close to the Rietveld workflow. The missing features can be overcome by using mercurial extensions. The expand lines can be solved by just looking at the repository code and/or apply the patch. The side-by-side diff can be fixed by applying the patch and use extdiff.
As mailing-list, I would choose: http://mlmmj.org/ because it works with email only, it is possible to request to resend a specific email (useful to make a review if you did not receive the email).
For flake8 check, we can have the bot subscribed to the ML. It will get the patch from the mail and use subject flag to find the repository. It will run `hg diff -c <rev>|flake8 --diff --show-source` and send a reply with the output.
For now, I see only one point to use Patchwork is to have a permanent link to the patch reviewed from the mailing list and add it to the bug tracker for example. But this could be solved if we have a mailing archive that allow to find email using their Message-Id, so we could just add such link to the bug-tracker.
I'd like to understand better your reservations...
(NB: I'm rather only in slight favour of rb)
> - another application to register and learn
What do you mean to 'register'?
If you mean 'login', it should be possible to integrate with the roundup
login mechanics quite easily. (see 'Writing Authentication Backends'
in the review board docs)
It is also rather trivial to learn.
Too, I believe most developers are rather adept as being faster learners
in this arena given the task at hand, it tends to be rather intuitive.
> - need customization to support nested patch
What do you mean by 'nested patch', recursive with diff -Nbur ?
can you be more specific?
> - UI more complex than Rietveld
Funny, at first look (without being able to log in) I thought the GUI in
Rietveld was somewhat more complex!
> - kind of overlap with the bug-tracker on some aspect.
Actually, rb can automatically link any bugs to the existing bug
tracker associated with the repository if this field is provided.
Quite nice as it can pick up the bug id out of the scm commit message for review.
Perhaps a useful way to evaluate is to see how some large projects use it,
like KDE or APACHE
Personally, I would prefer to keep the context provided with a non mailinglist based review system, which also keeps emails down to only notification traffic.
On 2017-09-10 19:49, richard wrote:
> > - another application to register and learn
> What do you mean to 'register'?
>
> If you mean 'login', it should be possible to integrate with the roundup
> login mechanics quite easily. (see 'Writing Authentication Backends'
> in the review board docs)
But this will be one more piece of code to maintain.
> It is also rather trivial to learn.
I do not find, Rietveld was already complex for many people, despite
being quite simple. Reviewboard has a bigger level of complexity.
> Too, I believe most developers are rather adept as being faster learners
> in this arena given the task at hand, it tends to be rather intuitive.
I do not understand.
> > - need customization to support nested patch
> What do you mean by 'nested patch', recursive with diff -Nbur ?
> can you be more specific?
See the nested option of hgreview.
> > - UI more complex than Rietveld
> Funny, at first look (without being able to log in) I thought the GUI in
> Rietveld was somewhat more complex!
I think we do not have the same definition of "complex".
More over, Reviewboard can not be used with keyboard.
> > - kind of overlap with the bug-tracker on some aspect.
> Actually, rb can automatically link any bugs to the existing bug
> tracker associated with the repository if this field is provided.
I can not find anything about roundup integration. And I doubt it exists
because of the design of roundup.
> Cédric Krier<cedric.krier@b2ck.com> added the comment:
> I would be in favor of mailing-list (with maybe Patchwork).
The main benefit I see using a mailing list is that we can also replace google-groups using the same mailing list server.
> Indeed using patchbomb (https://www.mercurial-scm.org/wiki/PatchbombExtension) is not so complicated.
Indeed this will be very similar to the current workflow, which is important for me.
> As mailing-list, I would choose:http://mlmmj.org/ because it works with email only, it is possible to request to resend a specific email (useful to make a review if you did not receive the email).
Why not using mailman? http://www.list.org/
It has more recent versions, more users/developers and is programmed in python.
> For flake8 check, we can have the bot subscribed to the ML. It will get the patch from the mail and use subject flag to find the repository. It will run `hg diff -c <rev>|flake8 --diff --show-source` and send a reply with the output.
Probably the same can be done to run a the test suite.
On 2017-09-20 11:57, Sergi Almacellas Abellana wrote:
> > Cédric Krier<cedric.krier@b2ck.com> added the comment:
> > I would be in favor of mailing-list (with maybe Patchwork).
> The main benefit I see using a mailing list is that we can also replace google-groups using the same mailing list server.
No, I do not want to manage a large mailing list.
> > As mailing-list, I would choose:http://mlmmj.org/ because it works with email only, it is possible to request to resend a specific email (useful to make a review if you did not receive the email).
> Why not using mailman? http://www.list.org/
>
> It has more recent versions,
Only of 1 day.
> more users/developers
The number of user should not be a primary criteria otherwise we will
not be on Tryton.
The criteria is about maintenance and mlmmj seems to be well maintained
also. But it is also about our capacity to solve problem and mlmmj has a
much more simple design than mailman.
> and is programmed in python.
Of course it would ease the maintenance but it will be easier to fix
something in mlmmj than in mailman because of the code complexity.
> > For flake8 check, we can have the bot subscribed to the ML. It will get the patch from the mail and use subject flag to find the repository. It will run `hg diff -c <rev>|flake8 --diff --show-source` and send a reply with the output.
> Probably the same can be done to run a the test suite.
El 20/09/17 a les 12:30, Cédric Krier ha escrit:
> On 2017-09-20 11:57, Sergi Almacellas Abellana wrote:
>>> Cédric Krier<cedric.krier@b2ck.com> added the comment:
>>> I would be in favor of mailing-list (with maybe Patchwork).
>> The main benefit I see using a mailing list is that we can also replace google-groups using the same mailing list server.
> No, I do not want to manage a large mailing list.
>
Then why we need to maintain a mailing list software? It won't be better to use google groups for patches also?
About patchwork, probably we can start without it and add it if needed.
And whatever the change we do, just make sure to update the how-to-contribute page from website and probably send a mail to tryton-dev explaining the changes too.
On 2017-09-20 16:14, Sergi Almacellas Abellana wrote:
>
> Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
>
> El 20/09/17 a les 12:30, Cédric Krier ha escrit:
> > On 2017-09-20 11:57, Sergi Almacellas Abellana wrote:
> >>> Cédric Krier<cedric.krier@b2ck.com> added the comment:
> >>> I would be in favor of mailing-list (with maybe Patchwork).
> >> The main benefit I see using a mailing list is that we can also replace google-groups using the same mailing list server.
> > No, I do not want to manage a large mailing list.
> >
>
> Then why we need to maintain a mailing list software? It won't be
> better to use google groups for patches also?
Because the goal is to not more depend on Google for contribution.
(Normally discuss should replace the google groups)
> About patchwork, probably we can start without it and add it if
> needed.
The main issue is to have a permanent link to the patch discussion.
> And whatever the change we do, just make sure to update the
> how-to-contribute page from website and probably send a mail to
> tryton-dev explaining the changes too.
The proposed solution may live in parallel with the current workflow. So
we can test first and change the default procedure later.
The next step to go forward on this issue is to write the reviewbot.
I think the reviewbot should be a script that can be run by procmail.
It should have a copy of all the repositories, find the right one using the parent hash of the patch, run flake8 on it and compose a respond email.
It is important that the script detect only new patches.
The repositories could be published somewhere on hg.tryton.org so we could have drone running test on it.
Would it not be possible to still setup an evaluation rb test server and/or perhaps kallithea in parallel for comparison (noticing http://repos.gewinnmonitor.de/grasbauer_reports in the recent tryton post)?
I still believe the rb work is quite trivial, for example, to support linking to the bugs.tryton.org issue, it should be nothing more than setting https://bugs.tryton.org/%s" in the bug url tab and I have a hunch that the authentication bits needed to use the round login is potentially as trivial (or more so) than a reviewbot script.