The document I wrote is hereby a commit guideline document, not policy. I'll retitle at a later point; it'll change the link. I'll rework a bit so that the tone doesn't sound too absolute.
Rob wrote: > Why should I wait weeks/months for some explicit review > Ask for a review, which as this document says is really just a LGTM threshold of approval, not even a real code review. Given your reputation of writing quality code, this isn't going to be an issue for you. If it's taking multiple weeks for anyone then we have a problem to fix -- and at present we do in Solr. Explicitly encouraging mere approvals (as the document says) should help a little. Establishing that we want this standard of conduct as this document says (even if not mandatory) will also help -- "you scratch my back, I scratch yours". But I think we should do even more... Gus wrote: > > 1. It would be nice if JIRA had a (*optional!*) PATCH_REVIEW status > (down stream from PATCH_AVAILABLE and it's infrastructure, but not > requiring PATCH_AVAILABLE as a prior step) that could be hooked up to send > a mail with an easily filterable subject line to individuals that have > (somehow) nominated themselves as interested in the associated Jira > Component > > I was thinking of this the other day. Hmm. Lets start a new thread on _how_ to get reviews; it's an important subject. The document encourages us to ask each other for reviews. Lets make this a habit. The health of the codebase is at stake. Rob then Thömas wrote: > > there is no "silence is consensus" > > Good point, maybe we should include something about this too. While > hopefully this doesn’t happen much, it doesn’t make sense to stall work for > weeks after putting up a final patch. Again, I think the doc’s intention is > to be a guideline, but if there are exceptions, so be it. > Yes, the doc should somehow make reference to and condone lazy-consensus. It's a last-resort but always an option. In Solr we need to take steps to get the reviews so that lazy-consensus is rare. For Lucene this is rare, I think. Rob wrote: > Please don't add unnecessary things to this document like "Linear git > history" which have nothing to do with code reviews. > I considered this. I think it's useful to have one document/page related to the guidelines of committing code with all that it entails. Many of the items are short and shouldn't get too long (I think). But I totally get your point that it's too much to discuss/debate at once. I will expressly mark those parts as "[PENDING DISCUSSION]" so we can focus on the review aspect now -- the most important topic. ~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley On Mon, Dec 2, 2019 at 1:18 PM Anshum Gupta <ans...@anshumgupta.net> wrote: > Thanks David, and every one else here. > > Let's not call this policy, as 1. there are folks other than me who think > that's too strong a word for what we are trying to do here, 2. this has > "rules" that we will have to ignore based on the circumstance. these rules > should instead be recommendations, and the policy be either what Robert or > Tomas suggested i.e. Recommendation/Guidelines. > > I feel that the document might be read in a different context by people > who weren't in the meeting. As far as I understand, the intention here is > to have better quality commits, with involvement of more than one person, > especially when the changes are large and affect the core/API. > > There are points around adding tests, which also directly are about just > improving code quality, something that has led to the failing tests and > other issues that have been spoken about in the recent past. > > +1 on adding something explicitly about "silence is consensus". > There are certain parts of the code that aren't worked by more than a > couple of people and in that case, these points will have to be just > interpreted based on the situation. > > Overall, I really think it will not only be awesome but is critical to > have more people reviewing code and for larger changes to be discussed > instead of being pushed in. > > > On Tue, Nov 26, 2019 at 11:04 AM David Smiley <david.w.smi...@gmail.com> > wrote: > >> Last Wednesday at a Solr committers meeting, there was general agreement >> in attendance to raise the bar for commit permission to require another's >> consent, which might not have entailed a review of the code. I volunteered >> to draft a proposal. Other things distracted me but I'm finally thinking >> of this task now. *This email is NOT the proposal*. >> >> I was about to write something from scratch when I discovered we already >> have some internal documentation on a commit policy that is both reasonably >> well written/composed and the actual policy/information is pretty good -- >> kudos to the mystery author! >> >> https://cwiki.apache.org/confluence/display/SOLR/CommitPolicy >> >> I'd prefer we have one "Commit Policy" document for Lucene/Solr and only >> call out Solr specifics when applicable. This is easier to maintain and is >> in line with the joint-ness of Lucene TLP. So I think it should move to >> the Lucene cwiki. Granted there is a possibility this kind of content >> might move into our source control somewhere but that possibility is a >> subject for another day. >> >> I plan to copy this to Lucene, mark as PROPOSAL and then make some large >> edits. The diff will probably be kinda unrecognizable despite it being in >> nice shape now. A "Commit Policy" is more broad that a "Code Review >> Policy"; it could cover a variety of things. For example when to commit >> without even filing a JIRA issue, which I think is worth mentioning. It >> should probably also cover Git considerations like merge vs rebase, and >> multiple commits vs squashing. Maybe we should also cover when to bother >> adding to CHANGES.txt and "via"? Probably commit message requirements. >> Snowballing scope :-). Probably not JIRA metadata as it's not part of the >> commit to be part of a commit policy, but _somewhere_ that's needed. I'm >> not sure I want to sign up for all that now but at least for the code >> review subject. >> >> ~ David Smiley >> Apache Lucene/Solr Search Developer >> http://www.linkedin.com/in/davidwsmiley >> > > > -- > Anshum Gupta >
