Tryton - Issues

 

Issue2178

Title Replace Rietveld
Priority feature Status chatting
Superseder Being less Google dependent
View: 2177
Nosy List Timitos, ced, nicoe, pokoli, risto3, semarie, xcodinas
Type feature request Components
Assigned To Keywords
Reviews

Created on 2011-10-10.20:23:19 by ced, last changed by semarie.

Messages
msg42071 (view) Author: [hidden] (semarie) Date: 2018-07-05.15:52:03
@pokoli: Oh, I missed it was already discussed.

About pathwork web interface: the public interface is mostly a static view. Patches and comments (replies on a thread with patch) are available with permalink. Message-ID is available too (could be used to create a link between mailing-list [web interface for archive reading] and patchwork, if the list interface provides such function [MHonArc is able to]). Patches are referenced by ID, but it is possible to reference them by hash.

patchwork provides several types of events: patch-created, patch-completed, patch-state-changed, ... (full list: https://patchwork.readthedocs.io/en/latest/usage/overview/#events) that could be used to automatically send mail on mailing-list (or to submitter) to advertize some changes for example.

But I am unsure to understand what you mean by "reply directly from the web interface". patchwork doesn't intent to replace the mailing-list. Discussions on patch should occurs on mailing-list. 
Eventually I could see some problem if you want to use patchwork webinterface for reviewing patches: you need to first search in your mailbox the related message to reply.
msg42068 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-07-05.11:48:46
@semarie: Patchwork was already pointed on msg30206. The main issue is it does not have an easy way to reply directly from the web interface. 

Has this option been added to newer versions?
msg42067 (view) Author: [hidden] (semarie) Date: 2018-07-05.10:22:04
another possibility (or a complementary one for mailing-list and reviewbot) is to use patchwork (http://jk.ozlabs.org/projects/patchwork/) for managing patches states on a mailing-list.

if I correctly understood:
- patchwork captures several types of mail from mailing-list: patches, or comments on patches (reply to a thread with patches) - it ignores others mails.
- patchwork stores patch + metadatas:
  - author, description, ...
  - state: for example "new", "rejected", "accepted"
  - tags: for example "Ack-by", "Tested-by", "Reviewed-by"
  - checks: for example: reviewbot-test, success, http://example.com/ID/test.log
- patchwork supports mercurial with patchbomb extension: it will create a serie of patches from multiple emails related (and the serie is manageable as a whole)
- patchwork accepts changes from command-line, mail, web
- patchwork generates events (acceptible from REST API): for example, an external tool could see when a new patch is arrived, run task on it, and update the patch state
- by using hook in the repository, a tool could update the state of patch when one is commited
msg41997 (view) Author: [hidden] (risto3) Date: 2018-06-30.11:59:40
Take a look at what mozilla (also a mercurial based project) are doing with reviewboard...
https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html
https://reviewboard.mozilla.org/

It seems all else is stalled.
msg38944 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-03-11.13:05:00
As it seems nobody spend the time to write the needed tools for mailing list review.

Another option would be to run Rietveld on AppScale: https://www.appscale.com/
The Foundation could rent a second DigitalOcean machine (like for discuss) and run AppScale using the docker image to simplify the maintenance: https://www.appscale.com/try-appscale
There is a documentation to migrate from GAE: https://github.com/AppScale/appscale/wiki/Migration-into-AppScale,-Backup-&-Recovery

So it needs to be tested and to see what happens to the existing users who are Google accounts when migrated as AppScale is designed to use on-premise: https://github.com/AppScale/appscale/wiki/Managing-Users
msg36299 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-10-16.09:10:59
We already not have the resource to setup one solution, I do not think we should try to spend to others.
msg36298 (view) Author: [hidden] (risto3) Date: 2017-10-16.07:02:43
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.
msg36292 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-10-15.23:06:48
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.
msg35708 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-09-20.16:55:07
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.
msg35704 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2017-09-20.16:14:43
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.
msg35693 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-09-20.12:30:06
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.

I do not see the point. Codestyle is not a bug.
msg35692 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2017-09-20.11:57:09
> 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.
msg35621 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-09-10.20:50:06
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.
msg35620 (view) Author: [hidden] (risto3) Date: 2017-09-10.19:49:55
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.
msg35619 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-09-10.12:14:01
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.
msg34902 (view) Author: [hidden] (risto3) Date: 2017-07-30.17:24:14
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).

Please do set up a test.
msg30395 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2016-11-22.18:00:01
I have used review board in a private environment and I can confirm the workflow is very similar to the rietveld one. 

Also having a bot is a very good feature. 

So I agree that it will be great to setup and instance and start to test it.
msg30206 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2016-11-06.17:09:32
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.
msg8857 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2011-10-10.20:23:19
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].

[1] http://svn.python.org/view/tracker/instances/python-dev/
History
Date User Action Args
2018-07-05 15:52:04semariesetmessages: + msg42071
2018-07-05 11:48:47pokolisetmessages: + msg42068
2018-07-05 10:22:04semariesetmessages: + msg42067
2018-06-30 11:59:40risto3setmessages: + msg41997
2018-05-11 22:36:30Timitossetnosy: + Timitos
2018-03-11 13:05:21cedlinkissue2177 superseder
2018-03-11 13:05:01cedsetmessages: + msg38944
2017-10-16 09:11:00cedsetmessages: + msg36299
2017-10-16 07:02:44risto3setmessages: + msg36298
2017-10-15 23:06:48cedsetmessages: + msg36292

Showing 10 items. Show all history (warning: this could be VERY long)