Guidelines for a Merge Request reviewer

[akouete-kpakpo]

Tasks

Document a checklist that a reviewer should follow before approving. Here are some suggestions:

• [x] check the context of the MR and validate the feature proposed solves the issue related

• [x] check the quality of the code: robust code, code linting etc

• [x] check if the feature does not break existing code by running in local + if there is a CI, request the proof that the pipeline of tests has succeeded on the working branch

[akouete-kpakpo]

Assignee @chrichri17 Reviewers: @abiolaTresor @Ethel2003

[chrichri17]

@abiolaTresor @Ethel2003 waiting for your review https://github.com/akouete-kpakpo/nevergrad/wiki/Team-conventions

[abiolaTresor]

@chrichri17 I think you should add something like "let the issue owner know when you've reviewed" or "communicate on your review". Not technical but very important still!

[akouete-kpakpo]

@chrichri17 I have some formatting comment. Could you split the bullet points in 2 sections: P0 & P1?

Apart from that , all good

[chrichri17]

@chrichri17 I think you should add something like "let the issue owner know when you've reviewed" or "communicate on your review". Not technical but very important still!

I don't think the reviewer should be doing this. I mean, as an assignee, I should always check the state of my PR and be reactive when I get new comments on my work. Does that make any sense @abiolaTresor ?

[akouete-kpakpo]

I should always check the state of my PR

well, @chrichri17 if you have many PRs it becomes difficult to follow. It is better if your reviewer tags you when he is done reviewing.

[abiolaTresor]

I agree with @akouete-kpakpo. @chrichri17 too complex for the assignee if he must permanently be tracking if reviewers have done their job

[akouete-kpakpo]

@chrichri17 please, is anything missing to close this issue?

[akouete-kpakpo]

@chrichri17 I'm closing. Feel free to reopen if something is missing.