Doesn’t this github review workflow as described work right now? It’s just not the “only” way people do things?
I don’t think we need to forbid other methods of contribution as long as the review and testing needs are met. -Jeremiah > On Jan 22, 2020, at 6:35 PM, Yifan Cai <yc25c...@gmail.com> wrote: > > +1 nb to the PR approach for reviewing. > > > And thanks David for initiating the discussion. I would like to put my 2 > cents in it. > > > IMO, reviews comments are better associated with the changes, precisely to > the line level, if they are put in the PR rather than in the JIRA comments. > Discussions regarding each review comments are naturally organized into > this own dedicated thread. I agree that JIRA comments are more suitable for > high-level discussion regarding the design. But review comments in PR can > do a better job at code-level discussion. > > > Another benefit is to relief reviewers’ work. In the PR approach, we can > leverage the PR build step to perform an initial qualification. The actual > review can be deferred until the PR build passes. So reviewers are sure > that the change is good at certain level, i.e. it builds and the tests can > pass. Right now, contributors volunteer for providing the link to CI test > (however, one still needs to open the link to see the result). > >> On Wed, Jan 22, 2020 at 3:16 PM David Capwell <dcapw...@gmail.com> wrote: >> >> Thanks for the links Benedict! >> >> Been reading the links and see the following points being made >> >> *) enabling the spark process would lower the process to enter the project >> *) high level discussions should be in JIRA [1] >> *) not desirable to annotation JIRA and Github; should only annotate JIRA >> (reviewer, labels, etc.) >> *) given the multi branch nature, pull requires are not intuitive [2] >> *) merging is problematic and should keep the current merge process >> *) commits@ is not usable with PRs >> *) commits@ is better because of PRs >> *) people are more willing to nit-pick with PRs, less likely with current >> process [3] >> *) opens potential to "prevent commits that don't pass the tests" [4] >> *) prefer the current process >> http://cassandra.apache.org/doc/latest/development/patches.html [5] >> *) current process is annoying since you have to take the link in github >> and attach to JIRA for each comment in review >> *) missed notifications, more trust in commits@ >> *) if someone rewrites history, comments could be hard to see >> *) its better to leave comments in the source code so people don't need to >> lookup github >> >> Here is how i see some of the points >> >> 1) I agree with the point that the high level discussions should be in >> JIRA; PRs are better at specific review and offer no real benefit over JIRA >> for larger structural changes >> 2) there are different patterns with multiple branches as well, but some of >> it is possible to codify and include in CI. For example, you could take >> the diff, attempt to apply to 2.2 (maybe if [dtest] in commit?) and forward >> merge; of any conflicts are found, could annotate JIRA that the change is >> complex and may be best to submit multiple PRs. Assuming we want something >> like this, it is also possible to run the tests against those branches as >> well. I am not saying we do this, but saying that it is possible to >> improve or solve this problem, so doesn't appear a blocker to me. >> 3) by marking it easier to comment i can definitely see this happen, but >> don't see this as a reason not to. I find that you are more willing to >> actually talk about small sections of the code in PR than in other forms >> and that its easier to track. One of the things i see now is that the >> conversation moves to slack, so is it better not happening, happening in >> slack, or happening in github? >> 4) This is actually why i started this thread. I created a patch a while >> back that passed review, got merged, and has been failing the build ever >> since. I would like to make it more clear that code is likely to do this >> or not. >> 5) The link documents the process as submitting patches generate by "git >> format-patch", which i was told not to do my first patch >> >> Think i summarized all I saw. >> >>> On Wed, Jan 22, 2020 at 2:30 PM Dinesh Joshi <djo...@apache.org> wrote: >>> >>> I personally use Github PRs to discuss the changes if there is feedback >> on >>> the code. The discussion does get linked with the JIRA ticket. However, >>> committing is manual. >>> >>> Dinesh >>> >>>> On Jan 22, 2020, at 2:20 PM, David Capwell <dcapw...@gmail.com> wrote: >>>> >>>> When submitting or reviewing a change in JIRA I notice that we have >> three >>>> main patterns for doing this: link branch, link diff, and link GitHub >>> pull >>>> request (PR); I wanted to bring up the idea of switching over to GitHub >>>> pull requests as the norm. >>>> >>>> >>>> Why should we do this? The main reasons I can think of are: >> consistency >>>> within the project, common pattern outside and inside Apache (not a new >>>> process for new members to learn), >>>> >>>> PRs are easier to review and comment on (much easier than linking lines >>> in >>>> a branch), Github and JIRA integration is already present so all >>>> conversations will be added to the JIRA work log, and could be linked >>> with >>>> Jenkins to trigger builds and tests and to report the status into JIRA. >>>> >>>> >>>> How would one start to do this? >>>> >>>> >>>> >>>> 1. Include the JIRA link in a commit message (example: >>>> CASSANDRA-<number> : message) >>>> 2. Create pull request (when creating the branch, the git message >>>> provides a link to create a pull request) >>>> >>>> >>>> That is it, by doing those two steps JIRA will be updated with all >> Github >>>> conversations, Jenkins could be notified and start building and report >>> back >>>> to JIRA. >>>> >>>> >>>> Thoughts? >>>> >>>> >>>> References: >>>> >>>> - https://www.apache.org/dev/svngit2jira.html >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >>> For additional commands, e-mail: dev-h...@cassandra.apache.org >>> >>> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org
