[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2512 Discussions related to that has been initiating several times, but if my memory is right, some of PMC members were not positive on this. This was regarding merge script but also contains squashing commits, and there was two -1s from PMC members with mentioning squashing commits. https://github.com/apache/storm/pull/1468 But I think things may be changed, so why not initiating another one? We have been discussing and changing legacy steps like removing contributors, removing CHANGELOG, etc. and I think this could be another one. ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2512 @hmcl I'd raise the discussion on the mailing list, then see if there's at least a lazy consensus. It seems likely to me that a discussion about this will peter out quickly, since some people probably won't care too much, but I think you only need for no one to object to a squash-all policy. If someone really doesn't like the idea, they will -1 and give their reasons. Otherwise I think this falls under non-code change in http://storm.apache.org/contribute/BYLAWS.html. If no one objects you could put a note about it in the DEVELOPER.md and other places committers are likely to see it. ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2512 @erikdw @srdo Thanks for checking in with me. I was off for the last few days and hence just getting back into this, although a bit too late because the patch just got merged before I got the chance to respond. @erikdw the fundamental reason behind squashing all the commits is sanity. The main challenge when the branches diverge is to track down what happened. The same happens when cherry-picking changes across branches. In Storm this is very hard because Storm's Git log does not have a wellâ defined structure, does not have one commit per JIRA, and contributors don't always squash their commits. As an illustration I am adding a the output of "git log" for Storm, Kafka, Spark, Hbase, Flink, and Apex. The difference is notorious. Every other project has a clean, consistent, Git log, which makes it very easy to understand and track down what happened. Furthermore, it makes it also very easy to write git hooks to automate all sorts of tasks to merge and track down what happens cross branches, for example, helping users that have their own private distribution which gets synced with Apache from time to time (I face such tasks periodically and understand how much easier it would be if had a clean git log). One could argue one way or another. However, a fact is that all of the aforementioned mainstream Apache projects follow one commit per JIRA (with the exception of having perhaps a consistent token such as MINOR), and commits always squashed. In my opinion we should do so as well with absolutely no exceptions. The extra effort to do this is minimal, and the benefits are immense. I have proposed this a few times before, but for some reason it has never been agreed upon or even warranted a lengthy discussion (which honestly is hard for me to understand why).  ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2512 @erikdw Yes, I think for cleanliness. Your reason for having two commits makes sense to me. I'll give Hugo a chance to respond, then I'll merge this in a day or two. ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user erikdw commented on the issue: https://github.com/apache/storm/pull/2512 @srdo : I assume for cleanliness / ease of reading purposes? I am a proponent of squashing commits when there are tons of slight tweaks being made to the same code. I do like separate commits for logically separate things, since it can help understand the code changes in more bite-sized chunks instead of mammoth unreadably-long commits. ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2512 @erikdw Makes sense. I brought it up because I believe at least @hmcl (can't recall if he's the only one) has been requesting that PRs are squashed before merging. Could you explain why @hmcl? ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user erikdw commented on the issue: https://github.com/apache/storm/pull/2512 @srdo : I actually spent extra effort to have 2 commits. :-) This was to ensure that there is no hidden changes amidst the renaming of the file (I am a strong proponent of trivial-to-read commits/diffs). With the 2 separate commits, in raw CLI `git` the changes are easy to follow. When I made the change at first in 1 commit I believe it was hard to see within the `git show` what lines had been touched. I might be misremembering a bit, but I definitely remember that it was clear in the `git status` that a non-content-changing rename was done, so long as I didn't touch the file contents simultaneously. ---
[GitHub] storm issue #2512: [STORM-2894] fix some random typos
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2512 +1. Could you squash to one commit before I merge this @erikdw? ---
