Taking care of infrastructure: code review

Not sure which are all the tools that you talk about. It will be great if you can list so we can discuss what to do in each case.

I see some tools will be replaced by gitlab:

  • codereview will be done on gitlab
  • No more needed to use a script (upload.py) to propose reviews.
  • flake8 checking can be done on CI (review-bot won’t be needed anymore)
  • drone will be replaced by gitlab CI (trypod won’t be needed anymore)

As far as we continue using mercurial the maintainer work should not change so much. Probably the only diference is how changes are pushed but they can also be done using standard mercurial tools.

2 Likes

All of them, looks at the menu of gitlab you will see.

More bad/low quality contribution means more work.

There is no need to migrate everything. We can just use gitlab for code hosting, codereview and CI. And continue using the same tools as we use now.

This lowers the learning curve to contribute to Tryton.

Is it really needed? For me the main blocker is that we are strict on the quality.

More bad/low quality contribution means more work.

But not all are developers, what about those who write the documentation?

Where are they? We already said that people can just send plain text and we will take care of formatting it.
For now we received 0 text.
Also you need to know deeply Tryton to write good and correct documentation.

Here is one example shared on this forum.

Of course the quality should be improved to land as oficial tryton documentation, but if we give the user the right tools and the needed comments I do not doubt they will suceed by providing good documenation.

Deeply nowing Tryton does not mean knowing how to develop Tryton and the tools used.

If we want to stay on Mercurial the best way to review is pre-commit reviews. Rietveld has the best usability of all tools I know, and there are not many tools around.
But Rietveld is no longer maintained. So we need to maintain at least its functionality by ourselves. This is a challenge, as we maintain already a lot of our infrastructure.

But IMHO all hassle we have, is because we use Mercurial in the first place.
We need pre-commit reviews only, because once a commit is done, it stays immutable in Mercurials commit history.
Other VCS like git have a different approach to immutability, which is not immutable per se. They use a Pull-Request workflow made of disposable dev-branches. And these VCS let the maintainers decide the quality of the commits and the history. Here the quality and consistence of the commit history depends on maintainer discipline.
There are many tools and web services like SCMs around the Pull-Request workflow and git. Once a pull request is “LGTM”, it can be squashed to one single commit and applied to the trunk.
Pull-Requests can be discussed line by line. And every iteration of the code evolution is visible in a new commit in the pull-request-dev-branch. IMHO it has a similar visibility and usability like the patch sets and the line comments in Rietveld.
The pull-request approach is very common to programmers nowadays.
But the git and pull-request-workflow is not for free: migration from mercurial and a new set of tools, routines and documentation is a challenge, too.

I constantly ask myself, which approach is better and more “future-prove”? What do you prefer?

2 Likes

And also does not work when using as many repositories as Tryton does.

What is the problem?

Pull requests only work for a single git repository, but currently Tryton uses Rietveld with patches that affect several repositories.

1 Like

I just found an interesting guide from mozilla explaining how they adapted phabricator review tool to suite its needs.

I have setup a phabricator instance some weeks ago. It has mostly all a project needs.

With mercurial codereview is not working without a local commit. For me that is ok because on the next change I ‘strip’ the last commit and do a new local commit. But it is not a ‘pre-commit’ like stated on there page.
But in phabricator you have all the tools together (CI, review, issue tracker, user management … ) - but you need to explore them.

FWIW, KDE uses gitlab, and it was really easy to contribute something (for me as a non-developer). Easier than github and Tryton-environment.

I identified a future problem with our infrastructure, I checked past proposal that did not happen and I made a proposal with a clear plan of action and a budget estimation.
Now I do not see the point to make another proposal if you do not propose a clear plan of action nor a budget because they can not be compared then.
I made some more researches and prototyping. I’m sure a review extension can be added and fully integrated to roundup without much work (not more than ±500 slocs) and without adding extra maintenance to roundup.

1 Like

Cost should not be the primarily driver. It is most likely that bolting something here and fixing something there is cheaper. You end up in an unmaintainable environment. Like many ERP customers who extended ‘just here and there’ a bit and now cant do upgrades any more. You know the game…

‘Not much work’ or ‘2 weeks effort’, as written in first post?

The current setup may fit perfectly to your way of working, but needs a huge learning curve for new contributors. It would allow sharing responsibilites, and it helps focussing on the goal to produce the worlds best OpenSource ERP system, instead of fixing things that are not maintained any longer upstream.

To sum it up in one sentence: hg is dead. Learn git.

I see no real proposal in your answer so I’m just ignoring it.

Why am I not really surprised by this answer?
My proposal is to think about a futureproof workflow for development. Gitlab may be one of them.
I have no experience in setting up gitlab (or other) but I trust that other community members have

As a daily user of mercurial and git I have to admit that mercurial is more intuitive than git. So I will be against a change in the version control system (also it has a cost with no benefit).

I may be but using the mercurial repositories thanks to heptapod.
Main problem is that currently there is no support for reviews on multiple projects and they do not consider it a feature to be implemented soon.

So I only see two options here:

  • Use a single repository with heptapod.
  • Develop our custom review project following @ced proposal.

I’m not very fan of developing a custom solution because it wil be to costly to have the same level of features of gitlab/heptapod (I’m thinking in web_ide). An standard solution can also be used for companies to develop it’s own modules following the same patterns and rules as in the tryton community easing the process to get familiar with the tryton contribution process.

Having said that I still do not have clear feeling about which is the best solution for the project.

I wasn’t aware that a blocker was a testing proposal.

I made a shell script which is doing such conversion:

  • take care of all open branches (but closed branches could be easily added)
  • keep all tags but rename them to avoid conflicts (“5.8.0” in “trytond/” is renamed as “trytond-5.8.0”)
  • keep the whole history (no commit rewrite)

there is one commit per subrepository to take care of tags rename + rebase under a directory. all subrepositories are merged (hg pull -f + hg merge)

some additional work might be necessary: for example tags aren’t 100% clean: some tags with old name are still here in duplicate of new names.

This is a first step but for me the complex and difficult tasks have always been:

  • make it works with CI (we can not run all the tests for each commits, it takes more than 8 hours now)
  • design new release scripts
  • readthedocs build

But even with all of that solved with a mono-repository, there are still the problem of the code review.