I disagree. I think we all agree there's value to having separate commits, but logical breakdown of a task often goes into detail that, when mirrored into JIRA without reason, is just busy work.

What value does an issue provide if there is no discussion and it is just a box to tick when working on another issue?

rb

On 05/11/2015 12:51 PM, Joe Witt wrote:
I was thinking the same thing Sean.

That said I think some of these things are 'advisories to make it
easier to bring in commits'.  But I'd be happy to not have them as
rules.  ...conservative in what we do and liberal in what we accept...

On Mon, May 11, 2015 at 3:47 PM, Sean Busbey <[email protected]> wrote:
Usually, when commits are better suited to be separate it means that there
should be additional jiras that are sub-tasks.

On Mon, May 11, 2015 at 2:18 PM, Ryan Blue <[email protected]> wrote:

I don't typically like to squash a series of commits, though I do
typically insist that commits are sensible before merging them.

The problem with squashing is that you end up with larger commits that are
more difficult to rebase or cherry-pick because they include more changes
than necessary. I'd much rather see a handful of focused commits that are
easy to reason about so when a cherry-pick conflict happens I have to worry
about fewer files.

That relies on a PR being made of a few "sensible" commits as I said
above, which basically means no "merge master into branch X" and requires
the submitter to force-push to their branch. I like to keep it that way,
but squashing avoids this problem.

rb


On 05/07/2015 12:45 AM, Joey Echeverria wrote:

I'm definitely a big fan of squashing before merging into develop. On
other projects, we usually don't bother squashing before the initial
PR, but I can see the advantage. After the first round of reviews has
happened it is nice to see diffs between the last state and the
current state so I try to avoid squashing again until the end.

Generally when you squash or rebase you have to do a force commit to
that branch. That shouldn't be a big problem if the branch isn't
shared. If you're working on a larger feature that has multiple JIRAs
all in the same branch then you have to be more careful with force
pushes.

-Joey

On Wed, May 6, 2015 at 5:03 PM, Sean Busbey <[email protected]> wrote:

A big reason I ask for squash commits is that it makes things much easier
once we have to maintain multiple release trains. It's just easier to get
in the habit now while we're small.

It's similar to the issue of making sure each issue has a jira reference
in
it. When I need to ask the question "is this issue fixed in release X"
it's
way way easier if I can know that when I see a commit with the jira, that
commit represents things are done rather than an incremental step.

Generally I use the short hand "one commit, one jira" for the combination
of making sure every commit references a jira and each jira can be seen
as
fixed by a commit. Taking this to the extreme of only allowing one commit
likely isn't wise. I've been on a few projects that will go for addendums
for a short window (say 24 hours) so long as they call out e.g. "NIFI-XXX
ADDENDUM forgot to use git-add while resolving a conflict." The thinking
is
that the addendum will likely be close in the history, will be uncommon,
and if the issue is backported afterwards to some other branch it can be
squished.

One related thing I saw recently was someone referenced the wrong jira in
their commit (or forgot the jira all together). It's unfortunate, but the
best option for this I've seen was on HBase: just revert the commit and
commit one with the correct information. Long term is eases more
headaches
then e.g. relying on a note in the jiras or something like that.

On Wed, May 6, 2015 at 7:42 AM, Joe Witt <[email protected]> wrote:

  Dan,

Yeah I was wondering about that too.  I suspect it is to make the
visual code review of the PR easier to digest.

We can document it on the Github/PR language but we also need to
produce a nice contribution guide that helps folks whether they're
coming from Github, are in the PPMC, and so on.

Thanks
Joe

On Wed, May 6, 2015 at 10:39 AM, Dan Bress <[email protected]>
wrote:

+1 on all this stuff.

As a side note, I noticed on a PR Busbey suggesting that people squash

their commits before submitting a PR.  I know I haven't been doing this,
and wouldn't mind hearing some pros/cons to doing this.   Whatever we
decide, can we include some language in the git hub / pull request
section
that describes the suggested approach.


Dan Bress
Software Engineer
ONYX Consulting Services

________________________________________
From: Mark Payne <[email protected]>
Sent: Wednesday, May 6, 2015 10:22 AM
To: [email protected]
Subject: Re: Consistency with Git

I vote +1 for these suggestions. I typically name feature branches
after

the feature, rather than the ticket. But I can see the advantage to
having
the ticket name in there as well. I like the suggestion to include both.



For release branches, I think this will help a lot. I’ve created and

destroyed several release branches trying to get this release done.
Having
the common parent would ensure isolation from develop but still allow
me to
easily destroy the branch rather than doing a release:rollback and
hoping
that all works well.



Thanks

-Mark






From: Joe Witt
Sent: ‎Wednesday‎, ‎May‎ ‎6‎, ‎2015 ‎9‎:‎57‎ ‎AM
To: [email protected]





Team,

It's been a few months and we started out with some ideas on how to do
things and each of us interpreted that slightly differently.  That
will continue to be true but we should document these things and keep
getting better and more consistent.  One area to consider then is the
usage of Git and our adherence to the Gitflow [1] workflow as we had
discussed.

In my opinion there is little call for us to deal with maintenance
branches.  So here i am just talking about 'feature branches' and
'release branches'.  What I'd propose is that we use what we've
learned and provide some better guidance on how to name the branches
and the life-cycle of them.

For 'feature branches':

- Recommend naming them 'NIFI-XYZ[-description]'  The [-description]
would be optional.  But for example this means the 'ListHDFS' branch
we have would have been NIFI-553-ListHDFS.

- This naming scheme helps people to know precisely which branch that
is about and it also promotes cohesive feature branches (that are
about a particular JIRA).

- Once the feature is complete and has been reviewed it can then be
merged into develop

- It isn't clear to me when it is the right time to clean up these
branches.  In my mind it seems like once the feature branch has been
merged to develop and part of a release then the feature branch can be
removed.  It isn't necessary for Git itself to do this but seems like
good housekeeping.

For 'release branches'

- Recommend naming them 'release-nifi-X.Y.Z'.

- This release branch would live on forever.  When generating an RC it
should branch from that release branch.  This way as RC's may come and
go we're not polluting the commit history.

- Once a given RC is accepted it can be merged back to the release
branch, master, and develop

- This extra sub-branch for the RC sounds a bit like overkill.  But it
exists to ensure that we do not pollute the commit history of the
release and beyond but also to ensure the community can keep
progressing with the develop branch.

- If for any reason we had to do an emergency type patch to a release
or whatnot we could do so with this branch and/or we can use the tag
which gets generated during the release process.

There needs to be more discussion around the entire lifecycle of
contribution of code that considers all roles of the community
including those submitting PRs from Github.  But this initial note is
just to get some consistency and open up for discussion.

[1]


https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow



Thanks
Joe





--
Sean



--
Ryan Blue
Software Engineer
Cloudera, Inc.




--
Sean


--
Ryan Blue
Software Engineer
Cloudera, Inc.

Reply via email to