Re: Discussion: reviewing larger tickets
While that approach optimises for people paying close attention to the JIRA firehose, it is less optimal for people trying to figure out after the fact what is going on wrt certain tickets. Some of the more complex tickets I cannot make head or tails of even when I was one of the main participants. It also doesn't promote translating these discussions into code comments for the permanent record. From my POV, though, I guess I can stick to my current approach and just cut/paste the results into JIRA if we want every nuance replicated there. On Thu, Jul 9, 2015 at 12:19 PM, Sam Tunnicliffe s...@beobal.com wrote: I'm +1 with Sylvain here; keeping the discussions open, accessible to all and persistent seems more valuable than reducing the friction for contributors reviewers. Personally, my workflow involves following the JIRA firehose, so I tend to be aware (at least to some degree) of both major minor comments, a lot of which I would surely miss were they to move GH. I also agree with the point that what seems minor to one viewer may raise red flags with another. That said, I often have offline conversations (from both the reviewer/contributor perspective) around minor-ish things like naming, nits and so forth. At the moment these are a) not recorded b) marginally more difficult to do asynchronously. So I think in future I may personally start using a GH branch for such remarks, though I don't think that should become a mandated part of The Process. Sam On Thu, Jul 9, 2015 at 11:47 AM, Sylvain Lebresne sylv...@datastax.com wrote: One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Yes, and I really worry about that. And I (see the I, that's a totally personal opinion) value keeping discussions as open as possible much more than additional convenience for making and processing review comments. I'll admit however that the current use of JIRA comments for reviews has never burden me all that much so I don't see all that much convenience to be gained by changing that process (but then again, I'm happy using vim for my development, so your mileage probably varies). Typically, if we talk of work-flow, I personally read JIRA updates fairly religiously, which allows me to keep vaguely up to date on what's going on with reviews even for tickets I'm a priori not involved with. I consider it a good, healthy thing. If we move some of the review material outside of JIRA, I strongly suspect this won't be the case anymore due to the burden of having to check multiple places. Anyway, I worry a bit that changing for what I perceive as relatively minor convenience will make us lose more than we get. Just my 2 cents however really. -- Sylvain On Wed, Jul 8, 2015 at 11:21 PM, Michael Shuler mich...@pbandjelly.org wrote: When we set up autojobs for the dev branches, I did some digging around the jenkins / githubPR integration, similar to what spark is doing. I'd be completely on board with working through that setup, if it helps this workflow. Michael On 07/08/2015 03:02 PM, Carl Yeksigian wrote: Spark has been using the GitHub PRs successfully; they have an additional mailing list which contains updates from GitHub ( http://mail-archives.apache.org/mod_mbox/spark-reviews/), and they also have their PRs linked to JIRA so that going from the ticket to the PR is easily done. If we are going to start using GitHub PRs to conduct reviews, we should follow similar contribution guidelines to what Spark has ( https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark#ContributingtoSpark-PullRequest https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark ), and have Infra set up the same hooks for our repo. We can also hook up cassci to do the same jobs as the AmplabJenkins performs currently. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the
Re: Discussion: reviewing larger tickets
I've started leaning towards a hybrid approach: I put everything I want to say, including some code changes, and sometimes complex argumentation into comments the branch. I differentiate these into two categories: 1. Literal comments, to remain for posterity - typically things I agree with, but for which it wasn't immediately clear I would at the outset; and 2. Queries/suggestions for the author, to be removed once they're resolved Then on JIRA, I make sure to raise explicitly for outside input, and for non-code-literate readers for posterity, any more major decisions that need consideration / discussion. Ideally, comments of type 2 would be replaced by summary comments of type 1, also for posterity. You can never have too many comments (so long as they're explanatory, not just restating the code, obviously) I think this probably leads to better JIRA and comments, as we: 1. Avoid the higgledypiggledy JIRA messes that can be very hard to unpick, consciously limiting discussion to high level decisions about approach, or unexpected complexities, etc. The things readers of JIRA care about. 2. Keep decisions about low level minutiae commented directly where they matter, to influence future authors without reference to JIRA On Wed, Jul 8, 2015 at 8:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie
Re: Discussion: reviewing larger tickets
Spark has been using the GitHub PRs successfully; they have an additional mailing list which contains updates from GitHub ( http://mail-archives.apache.org/mod_mbox/spark-reviews/), and they also have their PRs linked to JIRA so that going from the ticket to the PR is easily done. If we are going to start using GitHub PRs to conduct reviews, we should follow similar contribution guidelines to what Spark has ( https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark#ContributingtoSpark-PullRequest https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark), and have Infra set up the same hooks for our repo. We can also hook up cassci to do the same jobs as the AmplabJenkins performs currently. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie
Re: Discussion: reviewing larger tickets
The ability to navigate a patch in an IDE and add comments while exploring is not something the github PR interface can provide; I expect I at least would end up having to use multiple tools to perform a review given the PR approach. On Wed, Jul 8, 2015 at 3:50 PM, Jake Luciani jak...@gmail.com wrote: putting comments inline on a branch for the initial author to inspect I agree and I think we can support this by using github pull requests for review. Pull requests live forever even if the source branch is removed. See https://github.com/apache/cassandra/pull/4 They also allow for comments to be updated over time as new fixes are pushed to the branch. Once review is done we can just close them without committing and just commit the usual way Linking to the PR in JIRA for reference. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie -- http://twitter.com/tjake -- Joshua McKenzie DataStax -- The Apache Cassandra Company
Re: Discussion: reviewing larger tickets
(git history navigation is also much more powerful in the IDE, in my experience - can quickly scoot through many prior versions to see what the context of prior authors was) On Wed, Jul 8, 2015 at 9:15 PM, Benedict Elliott Smith belliottsm...@datastax.com wrote: Except that it would lack code navigation. So it would be alt-tabbing, then clicking through the clunky interface to find the file I want, and the location, which can be very cumbersome. On Wed, Jul 8, 2015 at 9:13 PM, Josh McKenzie josh.mcken...@datastax.com wrote: If you navigate in an IDE how do you know if you are commenting on code that has changed or not? I end up in the diff view and alt-tabbing over to the code view to look for details to navigate. In retrospect, working with a github diff would just be tabbing between a browser and IDE vs. an IDE diff and the IDE. On Wed, Jul 8, 2015 at 4:02 PM, Ariel Weisberg ar...@weisberg.ws wrote: Hi, If you navigate in an IDE how do you know if you are commenting on code that has changed or not? My workflow is usually to look at the diff and have it open in an IDE separately, but maybe I am failing hard at tools. Ariel On Jul 8, 2015, at 4:00 PM, Josh McKenzie josh.mcken...@datastax.com wrote: The ability to navigate a patch in an IDE and add comments while exploring is not something the github PR interface can provide; I expect I at least would end up having to use multiple tools to perform a review given the PR approach. On Wed, Jul 8, 2015 at 3:50 PM, Jake Luciani jak...@gmail.com wrote: putting comments inline on a branch for the initial author to inspect I agree and I think we can support this by using github pull requests for review. Pull requests live forever even if the source branch is removed. See https://github.com/apache/cassandra/pull/4 They also allow for comments to be updated over time as new fixes are pushed to the branch. Once review is done we can just close them without committing and just commit the usual way Linking to the PR in JIRA for reference. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie -- http://twitter.com/tjake -- Joshua McKenzie DataStax -- The Apache Cassandra Company -- Joshua McKenzie DataStax -- The Apache Cassandra Company
Re: Discussion: reviewing larger tickets
Except that it would lack code navigation. So it would be alt-tabbing, then clicking through the clunky interface to find the file I want, and the location, which can be very cumbersome. On Wed, Jul 8, 2015 at 9:13 PM, Josh McKenzie josh.mcken...@datastax.com wrote: If you navigate in an IDE how do you know if you are commenting on code that has changed or not? I end up in the diff view and alt-tabbing over to the code view to look for details to navigate. In retrospect, working with a github diff would just be tabbing between a browser and IDE vs. an IDE diff and the IDE. On Wed, Jul 8, 2015 at 4:02 PM, Ariel Weisberg ar...@weisberg.ws wrote: Hi, If you navigate in an IDE how do you know if you are commenting on code that has changed or not? My workflow is usually to look at the diff and have it open in an IDE separately, but maybe I am failing hard at tools. Ariel On Jul 8, 2015, at 4:00 PM, Josh McKenzie josh.mcken...@datastax.com wrote: The ability to navigate a patch in an IDE and add comments while exploring is not something the github PR interface can provide; I expect I at least would end up having to use multiple tools to perform a review given the PR approach. On Wed, Jul 8, 2015 at 3:50 PM, Jake Luciani jak...@gmail.com wrote: putting comments inline on a branch for the initial author to inspect I agree and I think we can support this by using github pull requests for review. Pull requests live forever even if the source branch is removed. See https://github.com/apache/cassandra/pull/4 They also allow for comments to be updated over time as new fixes are pushed to the branch. Once review is done we can just close them without committing and just commit the usual way Linking to the PR in JIRA for reference. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie -- http://twitter.com/tjake -- Joshua McKenzie DataStax -- The Apache Cassandra Company -- Joshua McKenzie DataStax -- The Apache Cassandra Company
Re: Discussion: reviewing larger tickets
When we set up autojobs for the dev branches, I did some digging around the jenkins / githubPR integration, similar to what spark is doing. I'd be completely on board with working through that setup, if it helps this workflow. Michael On 07/08/2015 03:02 PM, Carl Yeksigian wrote: Spark has been using the GitHub PRs successfully; they have an additional mailing list which contains updates from GitHub ( http://mail-archives.apache.org/mod_mbox/spark-reviews/), and they also have their PRs linked to JIRA so that going from the ticket to the PR is easily done. If we are going to start using GitHub PRs to conduct reviews, we should follow similar contribution guidelines to what Spark has ( https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark#ContributingtoSpark-PullRequest https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark), and have Infra set up the same hooks for our repo. We can also hook up cassci to do the same jobs as the AmplabJenkins performs currently. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie
Discussion: reviewing larger tickets
As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie
Re: Discussion: reviewing larger tickets
putting comments inline on a branch for the initial author to inspect I agree and I think we can support this by using github pull requests for review. Pull requests live forever even if the source branch is removed. See https://github.com/apache/cassandra/pull/4 They also allow for comments to be updated over time as new fixes are pushed to the branch. Once review is done we can just close them without committing and just commit the usual way Linking to the PR in JIRA for reference. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie -- http://twitter.com/tjake
Re: Discussion: reviewing larger tickets
If you navigate in an IDE how do you know if you are commenting on code that has changed or not? I end up in the diff view and alt-tabbing over to the code view to look for details to navigate. In retrospect, working with a github diff would just be tabbing between a browser and IDE vs. an IDE diff and the IDE. On Wed, Jul 8, 2015 at 4:02 PM, Ariel Weisberg ar...@weisberg.ws wrote: Hi, If you navigate in an IDE how do you know if you are commenting on code that has changed or not? My workflow is usually to look at the diff and have it open in an IDE separately, but maybe I am failing hard at tools. Ariel On Jul 8, 2015, at 4:00 PM, Josh McKenzie josh.mcken...@datastax.com wrote: The ability to navigate a patch in an IDE and add comments while exploring is not something the github PR interface can provide; I expect I at least would end up having to use multiple tools to perform a review given the PR approach. On Wed, Jul 8, 2015 at 3:50 PM, Jake Luciani jak...@gmail.com wrote: putting comments inline on a branch for the initial author to inspect I agree and I think we can support this by using github pull requests for review. Pull requests live forever even if the source branch is removed. See https://github.com/apache/cassandra/pull/4 They also allow for comments to be updated over time as new fixes are pushed to the branch. Once review is done we can just close them without committing and just commit the usual way Linking to the PR in JIRA for reference. On Wed, Jul 8, 2015 at 3:21 PM, Josh McKenzie jmcken...@apache.org wrote: As some of you might have noticed, Tyler and I tossed around a couple of thoughts yesterday regarding the best way to perform larger reviews on JIRA. I've been leaning towards the approach Benedict's been taking lately w/putting comments inline on a branch for the initial author to inspect as that provides immediate locality for a reviewer to write down their thoughts and the same for the initial developer to ingest them. One downside to that approach is that the extra barrier to entry makes it more of a 1-on-1 conversation rather than an open discussion via JIRA comments. Also, if one deletes branches from github we then lose our discussion history on the review process which is a big problem for digging into why certain decisions were made or revised during the process. On the competing side, monster comments like this https://issues.apache.org/jira/browse/CASSANDRA-6477?focusedCommentId=14617221page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14617221 (which is one of multiple to come) are burdensome to create and map into a JIRA comment and, in my experience, also a burden to map back into the code-base as a developer. Details are lost in translation; I'm comfortable labeling this a sub-optimal method of communication. So what to do? -- Joshua McKenzie -- http://twitter.com/tjake -- Joshua McKenzie DataStax -- The Apache Cassandra Company -- Joshua McKenzie DataStax -- The Apache Cassandra Company
