Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2550
@ptgoetz, @srdo and I have been using the following approach for squashing
the patches in storm-kafka-client, which I believe works well:
- It is reasonable to submit a PR for review that has a several commits,
typically done during development.
- During the PR review, changes addressing code review comments go in a new
commit such that it is easy to verify if the code review comments are property
addressed.
- Once the PR has a +1 from the reviewers and the changes are accepted to
merge the creator of the PR will squash the commits.
- I would vote for having only one commit with the commit message starting
either with STORM-1234: or MINOR:. I would favor not having commit messages
starting with arbitrary messages. However, this is a very debatable subject and
I am OK If we don't want to enforce that small commits have to be prefixed by
a token such as MINOR. I would say that significant patches should be required
to have the JIRA number at the top.
- I have discussed previously, and I believe that it is not beneficial to
to the community to merge into master PRs that have several commits that follow
a pattern along the lines:
```
STORM-1234: Some storm feature
fix compilation issue
address code review
fix checkstyle
fix unit tests
...
```
I think the entire community would benefit from a more structured way of
merging code into the main code line. Perhaps we could try again proposing some
PR and commit guidelines for voting. What do you think?
---
