Hi all, in principle I agree with Max. I personally avoid hotfixes and always open a PR, even for javadoc improvements.
I believe the main problem is that we don't have a clear definition of what constitutes a "hotfix". Ideally, even cosmetic changes and documentation should be reviewed; I've seen documentation added as a hotfix that had spelling mistakes, which led to another hotfix... Using hotfixes to do major refactoring or add features is absolutely unacceptable, in my view. On the other hand, with the current PR load it's not practical to ban hotfixes all together. I would suggest to update our contribution guidelines with some definition of a hotfix. We could add a list of questions to ask before pushing one. e.g.: - does the change fix a spelling mistake in the docs? => hotfix - does the change add a missing javadoc? => hotfix - does the change improve a comment? => hotfix? - is the change a small refactoring in a code component you are maintainer of? => hotfix - did you change code in a component you are not very familiar with / not the maintainer of? => open PR - is this major refactoring? (e.g. more than X lines of code) => open PR - does it fix a trivial bug? => open JIRA and PR and so on... What do you think? Cheers, -V. On 27 May 2016 at 17:40, Greg Hogan <c...@greghogan.com> wrote: > Max, > > I certainly agree that hotfixes are not ideal for large refactorings and > new features. Some thoughts ... > > A hotfix should be maven verified, as should a rebased PR. Travis is often > backed up for half a day or more. > > Is our Jira and PR process sufficiently agile to handle these hotfixes? > Will committers simply include hotfixes with other PRs, and would it be > better to retain these as smaller, separate commits? > > For these cosmetic changes and small updates will the Jira and PR yield > beneficial documentation addition to what is provided in the commit > message? > > Counting hotfixes by contributor, the top of the list looks as I would > expect. > > Greg > > Note: this summary is rather naive and includes non-squashed hotfix commits > included in a PR > $ git shortlog --grep 'hotfix' -s -n release-0.9.0.. > 94 Stephan Ewen > 42 Aljoscha Krettek > 20 Till Rohrmann > 16 Robert Metzger > 13 Ufuk Celebi > 9 Fabian Hueske > 9 Maximilian Michels > 6 Greg Hogan > 5 Stefano Baghino > 3 smarthi > 2 Andrea Sella > 2 Gyula Fora > 2 Jun Aoki > 2 Sachin Goel > 2 mjsax > 2 zentol > 1 Alexander Alexandrov > 1 Gabor Gevay > 1 Prez Cannady > 1 Steve Cosenza > 1 Suminda Dharmasena > 1 chengxiang li > 1 jaoki > 1 kl0u > 1 qingmeng.wyh > 1 sksamuel > 1 vasia > > On Fri, May 27, 2016 at 6:10 AM, Maximilian Michels <m...@apache.org> > wrote: > > > Hi Flinksters, > > > > I'd like to address an issue that has been concerning me for a while. > > In the Flink community we like to push "hotfixes" to the master. > > Hotfixes come in various shapes: From very small cosmetic changes > > (JavaDoc) to major refactoring and even new features. > > > > IMHO we should move away from these hotfixes. Here's why: > > > > 1) They tend to break the master because they lack test coverage > > 2) They are usually not communicated with the maintainer or person > > working on the part being fixed > > 3) They are not properly documented for future reference or follow-ups > > (JIRA/Github) > > > > That's why I have chosen not to push hotfixes anymore. Even for small > > fixes, I'll open a JIRA/Github issue. The only exception might be > > fixing a comment. It improves communication, documentation, and test > > coverage. All this helps to mature Flink and develop the community in > > a transparent way. > > > > I'm not sure what our contribution guidelines say about this but I > > would like to update them to explicitly address hotfixes. Let me know > > what you think. > > > > Best, > > Max > > >
