Hi again gem5 community! Sorry for sending the previous email at the beginning of the weekend! However, now that it's Monday, I want to this back up for discussion.
To give a bit more context, this proposal is meant to codify our current culture for code review. Almost everything proposed here is what most people try to do when submitting changes and reviews. However, it's never been "officially" written down anywhere. In keeping with this informal culture, *nothing* in this change will be enforced automatically. *The common case will be exactly the same as before* with reviewers enforcing our project culture. Our project is maturing. This means we need to do a better job of documentation, improve transparency, and, in some cases, slow down our changes to provide more stability for our users. This cannot be the job of one individual, one group, or one institution. If we want to see this project continue to grow, mature, and continue to be useful, the entire community needs to contribute to these goals. As always, I want to emphasize that gem5 is a meritocratic, consensus-based community project which values openness, transparency, and inclusiveness and strives to support its academic, research, and industrial users. Please review the changeset here: https://gem5-review.googlesource.com/c/public/gem5/+/36256 And, if you have time, there is another changeset trying to automatically assign reviewers which will also improve our reviewing capacity (we hope). See here: https://gem5-review.googlesource.com/c/public/gem5/+/34737 Cheers, Jason On Fri, Oct 16, 2020 at 3:50 PM Jason Lowe-Power <ja...@lowepower.com> wrote: > Hi all, > > I have just submitted a changeset to gerrit which updates the CONTRIBUTING > documentation with a proposal for updating the committing policies. > > Issue: https://gem5.atlassian.net/browse/GEM5-799 > Changeset: https://gem5-review.googlesource.com/c/public/gem5/+/36256 > > The gem5 project is maturing! We're getting more contributors and a higher > rate of contributions. Additionally, we are striving to provide stability > to our growing user base. > > In light of these changes, I am proposing updating our committing > guidelines for the review process for changes on gem5's "primary" > infrastructure. Details can be found on gerrit and are included below. > > This is currently just a proposal. I am looking for feedback from the > community. > > I believe the goals are: > - Provide transparency for changes that affect many users and ensure > community buy-in before making API changes > - Provide stability for users > - Provide documentation for API changes and new features > - Not increase the reviewing load significantly (in conjunction with the > new maintainer proposal from earlier today) > > The main changes are the following: > > For "primary" gem5 features at least two different people should review > the change before it is committed, except for trivial changes. There is no > automatic enforcement of this rule, but we expect the community to respect > this guideline. This guideline is meant to increase transparency in the > development process and prevent painful changes from affecting gem5's > users. > > "Primary" gem5 features are features used by a large number of users or > which affect many users indirectly. This will include most of the code in > base/, sim/, etc. This does not include legacy-supported ISAs (i.e., SPARC, > MIPS, POWER) or other subsystems that are not widely used (e.g., SystemC, > Arm Fastmodel, GPU VIPER coherence protocol, etc.). All APIs are "primary" > features. > > Before changing a gem5 API is it imperative to receive buy-in from the > gem5 community. The best way to do this is through communication with the > community before making the API change or after making a proposal for the > change. We ask that before any changes to the API are pushed to gerrit (or > at a minimum at the same time) the committer also does the following: > 1. Create an issue on the issue tracker. > 2. Send an email to gem5-dev mailing list > > We ask that these extra steps are taken for API changes to improve the > transparency and publicity of API changes. Not all users will track all > Jira issues or all commits pushed to Jira. Additionally, it is important to > provide the gem5 users with some stability to build on top of gem5. All > APIs are considered "primary" gem5 features and require at least two > reviewers. > > Additionally, APIs must be deprecated for at least two releases before the > old API is removed. The gem5 community is going through a transition to > increase its stability, and during this time, there may be some APIs that > have not been marked as such, but still require higher scrutiny before > committing. > > Finally, all API changes or additions require documentation. This > documentation requirement will be enforced during code review. > > Let me know what you think! Providing feedback on gerrit is preferred, but > high-level feedback via email is also great. > > Cheers, > Jason >
_______________________________________________ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
