Human Reviews of Pull Requests

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Human Reviews of Pull Requests

Guillermo Polito
Hi all,

As PRs are starting to stack, I think it would be good to gradually merge them. For that, I think it would be good to have both the automatic and a human review.

When you issue a PR to the development branch, you see two validations are launched:
 - basic validation
 - full validation

So far, they are in red because there are some failing tests (that we should fix BTW).

By now we can consider that a PR that runs and has some failing tests is OK. Actually, being able to run the tests means also the bootstrap had no problems. I'm tagging such PRs with human-review-needed in github:

https://github.com/pharo-project/pharo/pulls?q=is%3Apr+is%3Aopen+label%3Ahuman-review-needed

If gradually people can give their +1 or -1 to the PRs marked like that, we can quickly start integrating fixes.

Guille

--

   

Guille Polito


Research Engineer

French National Center for Scientific Research - http://www.cnrs.fr



Web: http://guillep.github.io

Phone: +33 06 52 70 66 13

Reply | Threaded
Open this post in threaded view
|

Re: Human Reviews of Pull Requests

alistairgrant
Hi Guille,

On Tue, Jul 11, 2017 at 09:30:02AM +0200, Guillermo Polito wrote:

> Hi all,
>
> As PRs are starting to stack, I think it would be good to gradually merge them.
> For that, I think it would be good to have both the automatic and a human
> review.
>
> When you issue a PR to the development branch, you see two validations are
> launched:
>  - basic validation
>  - full validation
>
> So far, they are in red because there are some failing tests (that we should
> fix BTW).
>
> By now we can consider that a PR that runs and has some failing tests is OK.
> Actually, being able to run the tests means also the bootstrap had no problems.
> I'm tagging such PRs with human-review-needed in github:
>
> https://github.com/pharo-project/pharo/pulls?q=
> is%3Apr+is%3Aopen+label%3Ahuman-review-needed
>
> If gradually people can give their +1 or -1 to the PRs marked like that, we can
> quickly start integrating fixes.
>
> Guille

Are there some guidelines published anywhere to help with performing the
review?

Thanks,
Alistair


Reply | Threaded
Open this post in threaded view
|

Re: Human Reviews of Pull Requests

abergel
In reply to this post by Guillermo Polito
Hi Guillermo,

Ronie has worked on porting a part of Torch to work with Git PullRequest.
Ronie, can you let Guillermo know how he can try your tool?

Alexandre
-- 
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.



On Jul 11, 2017, at 3:30 AM, Guillermo Polito <[hidden email]> wrote:

Hi all,

As PRs are starting to stack, I think it would be good to gradually merge them. For that, I think it would be good to have both the automatic and a human review.

When you issue a PR to the development branch, you see two validations are launched:
 - basic validation
 - full validation

So far, they are in red because there are some failing tests (that we should fix BTW).

By now we can consider that a PR that runs and has some failing tests is OK. Actually, being able to run the tests means also the bootstrap had no problems. I'm tagging such PRs with human-review-needed in github:

https://github.com/pharo-project/pharo/pulls?q=is%3Apr+is%3Aopen+label%3Ahuman-review-needed

If gradually people can give their +1 or -1 to the PRs marked like that, we can quickly start integrating fixes.

Guille

--
   
Guille Polito

Research Engineer
French National Center for Scientific Research - http://www.cnrs.fr


Phone: +33 06 52 70 66 13