Julian, it looks like you have not been using GitHub for the past 5 years, and you miss a lot of improvements. GitHub allows a superset of what we do in JIRA, so I really can't understand why do you suggest enforcing JIRA.
None of what you say sounds like a "good point to keep using JIRA", and none of what you say sounds like "a pitfall of using GitHub". >As to the backlog of PRs I could have just merged 2628, however: 1) We spend time on discussions in JIRA 2) After PR merge, someone would have to "resolve the JIRA" That consumes the reviewer's time. Julian>GitHub also makes it easy to write code, submit it as a PR, and that PR becomes the issue. That is a great GitHub feature. I do not see how do you treat it as something bad. It really makes no sense to create JIRA for adding tests, updating documentation. Julian>Lastly, another quibble about GitHub: that it allows people to squash, rebase and amend commits. The line-by-line comments quickly go out of date. squash, rebase, and amendments make JIRA comments go out of date at the same rate. However, in GitHub there's an explicit "resolve discussion", "reopen discussion" features, so there's a clear way to tell when the discussion is closed. JIRA has no conversation threads. GitHub has threads (code-related). Julian>GitHub makes it easy to review line by line, but you can then miss the important stuff. Do you mean we should stop code reviews? If we review code, then using both "JIRA and GitHub" makes it way easier to miss stuff. However, I agree we should review high-level ideas first, and put line-by-line comments only when high-level direction is OK. GitHub UI does have regular comments that are not attached to the diff lines, so you can comment there just like you add JIRA comments. Julian>And I like making holistic reviews What stops you from adding an overall review in GitHub? Julian>Then discussion can happen before coding starts. https://github.com/apache/calcite/pull/2628 is a perfect example here. The PR is trivial, and it adds a couple of tests. The PR looks good to me, except for method names. However, I truly do not know how to make test names better in Java, so I just ignore it. In Kotlin, one could use method name with spaces, so it would be better, however, I just ignored it. Then Julian comments "The test case names are not very useful". This should really belong to line-by-line diff rather than JIRA ticket. Now everyone should navigate between JIRA and PR to figure out the meaning. I see Julian suggests adding all the tests into a single method, however, I think we should just merge the thing and get over it. I am sure the discussion in CALCITE-4917 is not worth the time we spend there. We are discussing 2 lines of code there. Exactly two lines of test code. It would be OK to discuss method vs class when it comes to 100 lines. Even adding tests to JdbcTest does not have those levels of discussions. Vladimir
