Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider creating a more open review process #1130

Closed
strugee opened this issue Sep 1, 2015 · 6 comments
Closed

Consider creating a more open review process #1130

strugee opened this issue Sep 1, 2015 · 6 comments

Comments

@strugee
Copy link

strugee commented Sep 1, 2015

Obviously I'm not a core contributor, so feel free to disagree, but it seems like one of the biggest things holding this project back is lack of manpower. In particular, lack of manpower to review PRs.

I think that it might be a good idea for the Android team (which is mostly @davivel, AFAICT) to create a well-defined set of review guidelines that were clear to the point where a) some random contributor could self-review, thus reducing the cycles needed to get their PR merged, and b) community contributors like @tobiasKaminsky could review PRs, too.

If we had more people reviewing, it'd probably help a lot with the current "huge PR backlog" situation.

See also #1067

@tobiasKaminsky
Copy link
Contributor

This is a very good idea I think.
At least code formatting review can be done by all.
So we could add a new label: "ready for community review"
And this community review should be done by at least 2 persons.
After this review the label can change to "ready for review".

With these new labels the workflow can be described by labels as followed:
0 - Backlog
1 - Working
2 - Ready for community review
3 - Ready for review
4 - Ready for QA
5 - Approved by QA
After that: Remove labels and close issue

If @davivel agree we can collect in this issue the review guidelines?

@rperezb
Copy link

rperezb commented Oct 13, 2015

Thanks for the suggestions.

I have created new labels, mostly following the ones that are used on the core repo

0 - Backlog
1 - To develop
2 - Developing
3 - To Review
4 - To release

We do have also the Ready To Test, this I would say that it would be great to be used once the code has been reviewed and then we have the QA approved, which it´s mostly the "To release" one, isn't it?

@cmonteroluque your input it´s appreciated

@tobiasKaminsky
Copy link
Contributor

Although it is not the same as /core I would suggest to add the "ready to test" as 4 and when QA is finished go to "to release".
Then we would have an exact order and it is easier to find the labels as they are in the correct order on top of the list.

@tobiasKaminsky
Copy link
Contributor

Great, someone added "4 - ready to test" 👍

Next step would be to collect guidelines for reviewing process, or? (maybe in a new issue)

@ghost
Copy link

ghost commented Oct 14, 2015

@rperezb backlog should be a milestone, not a label, to match the other repositories.

@rperezb
Copy link

rperezb commented Oct 15, 2015

@tobiasKaminsky yes, I did it :), let´s leave open this issue till the reviewing process is updated, I will take care of in the following days
@cmonteroluque I have created the milestone as well as updated accordingly the iOS repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants