You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@storm.apache.org by erikdw <gi...@git.apache.org> on 2018/01/10 08:15:20 UTC

[GitHub] storm pull request #2512: [STORM-2894] fix some random typos

GitHub user erikdw opened a pull request:

    https://github.com/apache/storm/pull/2512

    [STORM-2894] fix some random typos

    * MultilangEnvirontmentTest
      * There is no "t" between n and m in Environment.
    * getOffsetFromConfigAndFroceFromStart
      * The r and o are transposed in Force.
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/erikdw/storm STORM-2894

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/2512.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2512
    
----
commit 7b849e20704fe6d51c01fd543fdf78f1989e04b4
Author: Erik Weathers <er...@...>
Date:   2018-01-10T08:08:24Z

    [STORM-2894] rename MultilangEnvirontmentTest.java to MultilangEnvironmentTest.java

commit 515b6492d2bc185f4454975d12edf6fe7fe14d22
Author: Erik Weathers <er...@...>
Date:   2018-01-10T08:09:00Z

    [STORM-2894] complete rename of MultilangEnvirontmentTest to MultilangEnvironmentTest, and fix typo of Force as Froce

----


---

[GitHub] storm issue #2512: [STORM-2894] fix some random typos

Posted by hmcl <gi...@git.apache.org>.

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).

![image](https://user-images.githubusercontent.com/10284328/34959386-299e7800-f9eb-11e7-99a5-9b18022e6665.png)

---

[GitHub] storm issue #2512: [STORM-2894] fix some random typos

Posted by erikdw <gi...@git.apache.org>.

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

Posted by erikdw <gi...@git.apache.org>.

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.


---