Working with code review

Support Developer
1

I did not understand the ideal/recommended workflow with code reviews:

  1. I cloned the tryton environment:
    hg clone https://hg.tryton.org/tryton-env
  2. I made some changes.
  3. I push them for a review:
    python ~/.local/bin/upload.py -t "comment ...." -i XXXXX
  4. I make other changes, how to push them in another review ?

Should it commit after point 2 on my environment? if yes how to push this commit in the corresponding review and push the 2nd modification in the other review?
Do I need to clone an environment by modification?

2

For me, it depends if changes concerns just a module or multiple modules…
You must not commit.
Indeed if your review concerns different modules, you could work in the same environment. (just do your upload in the module you’re working at)
But best (to avoid errors or to be lost in your issues) is to have one directory for each issue…
I asked the same question to @nicoe during the code sprint in Marseille. Don’t know if there’s some infos about that in discuss.

3

When I upload my modifications I am not in a module or other directory but at the root of ‘tryton-env’, all my modifications are therefore affected by the upload (unless I do it wrong :frowning: )

4

If you upload your modifications at the root, indeed all your modifications will be uploaded.
But let’s say you work on two different reviews: one concerning module sale and another one concerning purchase, you have to launch your command upload.py in the directory of the module. (then only the changes in the module will be affected).

5

The way I work is by having a directory tryton-env with a repository tryton-env in it.
Then for each fix / development, I clone this repo with a name like SaoBoolDirectClick-issue9285.
In this directory I usually keep a file review_id with the review issue number in it so that I don’t have to remember its number.

This is my workflow, it might not suit you but it might help you.

1 Like
6

I will share how I work in case helps anybody else:

  • I have a local copy of the tryton-env.
  • I develop patch and I upload them to codereview.

I use curl to download the raw version of a codereview and I apply it loacaly. For example:

$ curl https://codereview.tryton.org/download/issue293541002_293721003.diff | patch -p1

I used the raw link in the codereview patch to get the proper download url.

Note that if you want to apply a patch, you have to revert any local patches first. This can be done with hg revert, which will revert all patches in all modules if you execute it from the root folder.

There are some speciall cases, normally when developing a new module, that I follow the same aproach than Nicolas. But for small contributions I use the main repository.

Hope it helps!

1 Like
7

It does not look like a very good way as you seem to forget to add new files :yum:.

I think people should realize that the default unnamed branching model of mercurial is to have a local clone. It is very fast and not expensive because hard-links are used.

1 Like
8

Well it seem’s i’m not the only one :stuck_out_tongue_winking_eye:

Agree that this is very fast but having to create a new environment and run each patch in it’s own server has it’s own complexity.

I prefer to have a mixed approach to combine both despite I sometimes upload unrelated changes :shushing_face: :shushing_face:

9

I do not understand why you need to create a new environment. You can reuse the same virtualenv for all clones.

10

Because I install the trytond and proteus projects from sources (pip install -e) so I do not need to take care about paths and because I can run the test suite from any directory.

I’m used to run the test suite without closing the editor.

11

I do not know very well Mercurial, but is there not a way to do as with the branches of git. A branch by modification and we can choose to merge or not the branches towards a new branch for our tests and we can choose that modification to push for review ?

12

You might be interested in the different branching models available in mercurial to choose the one you prefer:

https://stevelosh.com/blog/2009/08/a-guide-to-branching-in-mercurial/

For Tryton’s code we’re keeping a linear tree so rebasing should be used.

13

How do you import a review when the development repository has advanced?
Currently I process rejects by hand but isn’t there a better method?

14

You have to pull the changes from the oficial repository and resolve the conflics.

If your does not apply a .rej file will be generated with the part that can not be correctly applied. You should use this files as reference to re-apply the patch.

Once everything is fixed you should reupload the patch to the review server.

HIH

15

Ok thanks …

… what pity, that there is no magic command :stuck_out_tongue_winking_eye: