This is based on LLVM's Code Review policy. It differs it not
requiring review for non-trivial changes.
Details
- Reviewers
imp jhb allanjude - Group Reviewers
Core Team - Commits
- rD52815: Committers Guide: Add a section encouraging pre-commit review.
Diff Detail
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 18822 Build 18486: arc lint + arc unit
Event Timeline
Looks good to me. Not sure if we want to encourage smaller patches where it makes sense, or if that's covered elsewhere.
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
2365 | Is there some way to suggest people work in small patch chunks so they can easily generate the small patches? |
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
2376 | The 'an appropriate commit bit access' phrase reads a bit odd to me. Perhaps either 'an appropriate commit bit' or 'appropriate commit access'? |
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
2376 | I'm not sure about this detail... There are times that I include non-committers on the review list because I know they are domain experts for something or another. This happens more with ports than with src since most domain experts are already src committers. I suspect this will cause heart-ache when we post it to developers. Also, I wouldn't think of committing a non-trivial vm fix w/o running it by alc, kib, jeffr, etc and having at least one of them sign off on it. I would accept the review from others, but I wouldn't consider even jhb's 'approval' of a phab review to be enough to commit it, even though he has appropriate commit access. I'm at a lost, though, for how to cast this into words we can use here. |
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
2376 | We could say "an appropriate subject-matter expert (usually a committer)." which would let use handle things like numerics where the de facto maintainer resigned his commit bit. I might also add something like: Some areas of the tree have more strict review requirements, e.g areas documented in the MAINTAINERS file of the source tree or ports infrastructure in the Mk subdirectory. |
- Fix unicode apostrophes.
- Convey that reviewers can't usually be any random committer.
- Rebase
- Clarify that these are most specific to src.
- Provide an out for changes where there isn't an expert available.
Just a small typo.
en_US.ISO8859-1/articles/committers-guide/article.xml | ||
---|---|---|
2323 | s/Phabrictor/Phabricator/ |