Squash and Merge for PRs

[Dave Neuman <ne...@apache.org>]

Hey All,
I want to follow up on something that was briefly discussed at the summit
this week.  Many people do not like the Changelog and feel like it can be a
PITA to deal with.  One of the reasons we have the changelog is because
there are so many commits in a given release, that it is hard to comb
through all of them to figure out what noteworthy changes or bug fixes went
into the code.  One thing that may help with this problem is to use enable
the squash and merge feature on Github for pull requests [1].   This
feature would squash all commits in a PR into one commit.  If we pair the
one commit with a good commit message, we would essentially get the ability
to create the changelog just from the commits.

So, I would like to propose that we enable the squash and merge feature in
the Traffic Control Github repo and use that going forward.  Thoughts?

Thanks,
Dave


[1] https://help.github.com/articles/about-pull-request-merges/

Re: Squash and Merge for PRs

[Leif Hedstrom <zw...@apache.org>]

> On Oct 18, 2018, at 7:40 AM, Dave Neuman <ne...@apache.org> wrote:
> 
> Hey All,
> I want to follow up on something that was briefly discussed at the summit
> this week.  Many people do not like the Changelog and feel like it can be a
> PITA to deal with.  One of the reasons we have the changelog is because
> there are so many commits in a given release, that it is hard to comb
> through all of them to figure out what noteworthy changes or bug fixes went
> into the code.  One thing that may help with this problem is to use enable
> the squash and merge feature on Github for pull requests [1].   This
> feature would squash all commits in a PR into one commit.  If we pair the
> one commit with a good commit message, we would essentially get the ability
> to create the changelog just from the commits.


Why not generate the ChangeLog from the actual PR “subject” line? Then it wouldn’t matter if you have 1 or 100 commits for a particular PR. The Sorber wrote a little script for ATS that generates our ChangeLogs *).

That much said, there’s another really good reason to squash: it makes back porting / cherry-picking fixes a hell of a lot easier.

Cheers,

— Leif

*) https://github.com/apache/trafficserver/blob/7.1.x/CHANGELOG-7.1.5

> 
> So, I would like to propose that we enable the squash and merge feature in
> the Traffic Control Github repo and use that going forward.  Thoughts?
> 
> Thanks,
> Dave
> 
> 
> [1] https://help.github.com/articles/about-pull-request-merges/

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Dave Neuman <ne...@apache.org>]

Hey all,
I am considering this conversation done and consensus reached.  I have
submitted a ticket to the infra team to have squash and merge enabled on
our repo.  The ticket can be found here:
https://issues.apache.org/jira/browse/INFRA-17227

Thanks,
Dave

On Thu, Oct 25, 2018 at 8:57 PM Jeremy Mitchell <mi...@gmail.com>
wrote:

> After reading Chris' quite detailed response :) I'm leaning towards `Squash
> and Merge` but the PR needs to be as atomic as possible. And we should
> expect committers to enforce this. I.e.
>
> some people have PR's that look like this:
>
> PR title: Adds Feature X
> - commit 1 - cranking out some API endpoints
> - commit 2 - writing some API tests
> - commit 3 - cranking out some UI code
> - commit 4 - writing some UI tests
> - commit 5 - Adding some database tables
> - commit 6 - writing some documentation for feature X
>
> some people have PR's that look like this:
>
> PR title: Adds Feature X
> - commit 1 - Adds Feature X
>
> regardless of the approach, squash that down into one commit with message
> of `Adds Feature X`
>
> Hopefully, that will result in smaller PR's that can get merged faster.
>
> +1 squash
>
>
>
> On Wed, Oct 24, 2018 at 5:12 PM Chris Lemmons <al...@gmail.com> wrote:
>
> > Right, the primary difference between "squash and merge" and "rebase
> > and merge" is who is responsible for writing the commit message.
> > Squash asks the committer to do it, as part of the squash. Rebase asks
> > the author to do it, as part of the commits in the PR. Generally
> > speaking, I think authors should be responsible for their own commit
> > messages. It can be a bit tough to find committers for merging PRs as
> > it is, I don't know that asking folks to write commit logs for
> > everything they merge is likely to improve things. :/ In my mind, a
> > really solid PR process example might look like this:
> >
> > 1. Author implements a moderately sized feature.
> > 2. The author creates commits, one for each major atomic unit of code
> > for the feature (usually, just one commit). The commit messages have
> > subjects and bodies, like I mentioned before. Each commit has all the
> > relevant testing and documentation that is associated with the code it
> > contains.
> > 3. The author creates a PR. The subject of the PR is the broad feature
> > title. The body contains the information we use the template to ask
> > for.
> > 4. A committer comes by and identifies areas for improvement. (A
> > non-committer may also join the review, but at least one committer is
> > involved, naturally.)
> > 5. The author improves those areas until everyone reaches a consensus
> > on the mergeability.
> > 6. If there were improvement commits, the author does a `rebase -i` to
> > squash those improvement commits appropriately up into the originals,
> > leaving the same set of commits the PR started with, but better than
> > before, because of the review.
> > 7. If, during this time, master has drifted so that the branch no
> > longer rebases cleanly, the author also rebases onto master and fixes
> > the conflicts.
> > 8. The author force-pushes to the branch.
> > 9. A committer does one last sanity check to make sure the commits
> > look right and hits "rebase and merge".
> >
> > For comparison, here's an example of what I think a squash process
> > might look like:
> >
> > 1. Author implements a moderately sized feature.
> > 2. The author creates commits as they go, with mildly useful, but not
> > really informative commit messages.
> > 3. The author creates a PR. The subject and body contain the
> > information one might expect in the subject and body of an atomic
> > commit.
> > 4. A committer comes by and identifies areas for improvement. (And
> > non-committers, as appropriate.)
> > 5. The author adds commits to the PR to address things until consensus
> > is reached.
> > 6. If master has drifted, the author rebases onto master and
> force-pushes.
> > 7. The committer hits "squash and merge" and, using the PR title and
> > body as a guide, possibly verbatim.
> >
> > The squash and merge require slightly more of the committer and
> > slightly less of the author, and it doesn't work if the unit of work
> > isn't a single atomic unit of code. But for most of our work, I think
> > it amounts to the same thing.
> >
> > And then, we can use `git log` to produce changelogs. If we really
> > want to, we could use the standardized words from Keep a Changelog to
> > identify kinds of commits:
> >
> > > "Added" for new features.
> > > "Changed" for changes in existing functionality.
> > > "Deprecated" for soon-to-be removed features.
> > > "Removed" for now removed features.
> > > "Fixed" for any bug fixes.
> >
> > Ultimately, PRs are transitory, so I'd really like for any genuinely
> > important information to make its way into a commit message that will
> > stick around for a while.
> > On Wed, Oct 24, 2018 at 10:44 AM Jeremy Mitchell <mi...@gmail.com>
> > wrote:
> > >
> > > I do like the idea of adding the 'squash and merge` option so we end up
> > > with 2 Github options:
> > >
> > > - rebase and merge
> > > - squash and merge
> > >
> > > Leave it up to the merger which option to choose.
> > >
> > > Now as far as how to create a "change log", back to what Leif said
> > earlier:
> > >
> > > "Why not generate the ChangeLog from the actual PR “subject” line? Then
> > it
> > > wouldn’t matter if you have 1 or 100 commits for a particular PR. The
> > > Sorber wrote a little script for ATS that generates our ChangeLogs *)"
> > >
> > >
> > >
> > > On Wed, Oct 24, 2018 at 10:24 AM Rawlin Peters <
> rawlin.peters@gmail.com>
> > > wrote:
> > >
> > > > I agree with most of this and wish everyone would structure their PRs
> > > > and commits like this, but the fact of the matter is that small
> > > > checkpoint commits are simply just the way some people like to work.
> > > > For the last few things I've worked on, I've tried to separate
> changes
> > > > to the different components for the same logical feature into
> > > > different PRs (API, UI, Traffic Router). I find that breaking down
> the
> > > > feature into a PR per component makes things a bit easier to review,
> > > > since not everyone has experience all the various TC components that
> > > > could be mashed into one mega-PR. If a mega-PR contains changes to
> TO,
> > > > TP, and TR all in one, some people maybe be comfortable reviewing TO
> > > > but not necessarily TP or TR, so you end up having to coordinate with
> > > > multiple reviewers/committers who are comfortable reviewing different
> > > > areas just to get a single PR merged.
> > > >
> > > > Unless we start making good commit messages and "one standalone
> > > > change" per commit a hard requirement for all contributors to the
> > > > project, I doubt we can ever get to this ideal, and I'm not sure we
> > > > should make that a requirement right now. However, by squashing and
> > > > merging commits, we can clean up the git history by removing all of
> > > > the non-atomic/checkpoint commits as well as other benefits that have
> > > > already been discussed.
> > > >
> > > > Nonetheless, we will still be able to use the "rebase and merge"
> > > > option at the committer's discretion. If the contributor and
> committer
> > > > feel that the commits in the PR are more valuable in the history
> > > > because they're atomic and each have good commit messages, the
> > > > committer should choose the "rebase and merge" option. If the PR
> > > > contains lots of small, nonatomic, checkpoint commits that would
> > > > obfuscate the git history and make it harder for someone doing a `git
> > > > blame` to figure out why a change was made, then maybe it would be
> > > > better for the committer to choose the "squash and merge" option or
> > > > kindly ask the PR submitter to squash the commits themselves into a
> > > > single commit and write a good title+message.
> > > >
> > > > - Rawlin
> > > >
> > > > On Tue, Oct 23, 2018 at 4:49 PM Chris Lemmons <al...@gmail.com>
> > wrote:
> > > > >
> > > > > I don't think I'm quite -1 on this, but I think we can do better.
> > > > >
> > > > > I think a PR should be a single unit of work and a commit should
> be a
> > > > > single unit of code.
> > > > >
> > > > > I like using atomic commits. That is, each commit does one thing.
> So
> > > > > if I need to revert a fix or track down when something was
> > introduced,
> > > > > I can find it. For commit messages, I really like the seven rules
> > > > > documented here, but especially the last one:
> > > > > https://chris.beams.io/posts/git-commit/
> > > > >
> > > > > > Use the body to explain what and why vs. how
> > > > >
> > > > > I think it's important to put the body in the commit message in
> order
> > > > > to explain why the change is necessary what the non-obvious effects
> > of
> > > > > it are. Having a good subject for the commit is also very useful,
> > > > > since that's what GitHub (and really all the tools) shows when you
> > > > > scroll through commits, looking for what changed.
> > > > >
> > > > > In order to have atomic commits, sometimes you'll need one or more
> > > > > commits as part of a single PR. Not most of the time, but
> sometimes.
> > > > > Especially with a large feature, you may make multiple dependent
> > > > > changes, but each one is a logical unit of work. For example, if
> the
> > > > > task is "add support for foobaz plugin", there might be three
> > commits,
> > > > > with summaries that look like "Add support for foobaz plugin to
> > config
> > > > > generation", "Add UI for editing foobaz plugin config", and "Add
> API
> > > > > endpoints for foobaz plugin config". That also makes it easy to
> > direct
> > > > > domain experts to each of the changesets for focused review.
> > > > >
> > > > > A PR on the other hand, is a single unit of work. While each commit
> > > > > should be stand-alone, capable of running correctly before and
> after,
> > > > > each commit might not be desirable on a standalone basis. For
> > example,
> > > > > while "Add support for foobaz plugin to config generation" is a
> > single
> > > > > unit of code in that it operates correctly, has tests, is
> documented,
> > > > > and is completely implemented, it's not exceptionally useful
> without
> > a
> > > > > UI to edit it. It makes sense to group all these changes and merge
> > > > > them as a unit.
> > > > >
> > > > > For almost all bug fixes and most medium to small features, each PR
> > > > > will have precisely one commit. But especially for larger
> "projects",
> > > > > it makes the most sense to group it all as one PR, but merge it as
> a
> > > > > feature.
> > > > >
> > > > > Relatedly, it's very rarely correct to merge commits from two
> > > > > different authors. (This is important for legal reasons as well, if
> > it
> > > > > ever came to it; authors license their code when they contribute
> it,
> > > > > so it's important to know whence it came.) However, in the event
> that
> > > > > two people jointly produce what amounts to a single unit of code
> (for
> > > > > example, by pair programming, or by collaborating on a snippet of
> > code
> > > > > on Slack or IRC), you can document that in git by adding it to the
> > end
> > > > > of the commit message body:
> > > > >
> > > > > > Co-authored-by: name <na...@example.com>
> > > > > > Co-authored-by: another-name <an...@example.com>"
> > > > >
> > > > > Details for that technique here:
> > > > >
> > > >
> >
> https://help.github.com/articles/creating-a-commit-with-multiple-authors/
> > > > >
> > > > > It's not quite a "standard", but it's quickly being adopted by most
> > of
> > > > > the tools I use, at least.
> > > > >
> > > > > As for the "Squash" option, I actually prefer the "Rebase" choice.
> > > > > It's the same thing if you have single-commit PRs, but it handles
> > > > > multi-commit PRs a bit more nicely.
> > > > >
> > > > > So, to summarize, here's what I think:
> > > > >
> > > > >  - Each commit should be an atomic unit of code and be described
> by a
> > > > > descriptive commit subject and body.
> > > > >  - Each PR should be a logical grouping of closely related work;
> > > > > usually a single commit.
> > > > >  - We should use Rebase for flattening our changes.
> > > > >  - Squash is still an "okay" heuristic that accomplishes the same
> > > > > thing, most of the time.
> > > > > On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <
> > dewrich@apache.org>
> > > > wrote:
> > > > > >
> > > > > > +1 based upon the fact that the Changelog.md can't seem to stay
> > > > current, we
> > > > > > do need to be able to maybe at least generate one from the commit
> > > > messages
> > > > > > within that release window.
> > > > > >
> > > > > > -Dew
> > > > > >
> > > > > > On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <
> > > > Jonathan_Gray@comcast.com>
> > > > > > wrote:
> > > > > >
> > > > > > > +1
> > > > > > >
> > > > > > >  In a past life when using Microsoft VSTS with git, the default
> > > > action was
> > > > > > > squash.  The rule of thumb we used was squash by default and
> only
> > > > merge if
> > > > > > > every commit was meaningful and required to understand what you
> > were
> > > > trying
> > > > > > > to do.  We didn't typically have communal feature branches, so
> it
> > > > wasn't as
> > > > > > > large a use case for us.  The repo forensics are nice with
> merge,
> > > > but the
> > > > > > > reality we found was that git blame was more useful when we
> could
> > > > track to
> > > > > > > the PR rather than a mid-stream commit and then having to make
> a
> > > > second
> > > > > > > jump to the PR from there.  It also cut down on back/forth
> design
> > > > change
> > > > > > > commits and helped us stay on track with 1 PR == 1 issue
> because
> > it
> > > > made
> > > > > > > unrelated fixes more apparent.
> > > > > > >
> > > > > > > Jonathan G
> > > > > > >
> > > > > > >
> > > > > > > On 10/23/18, 12:40 PM, "Fieck, Brennan" <
> > Brennan_Fieck@comcast.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > >     I'm not gonna say +1 or -1, but I'd still like to chime
> in. I
> > > > think
> > > > > > > it'd be nice to require PRs to squash things into atomic
> chunks.
> > No
> > > > commits
> > > > > > > that say "started x" or "minor fixes", but idk if it's
> > reasonable to
> > > > be
> > > > > > > able to boil every PR forever down to 70 characters. And sure
> you
> > > > can make
> > > > > > > multiple PRs if changes are too big to fit in one commit
> message,
> > > > but then
> > > > > > > it becomes really easy to become entwined in a chain of
> dependent
> > > > PRs so
> > > > > > > that every time one of them gets merged you have to rebase them
> > all.
> > > > > > >
> > > > > > >     Of course, a "reasonable squash" is pretty hard to define
> > > > absolutely,
> > > > > > > and could actually just make PRs harder to review -  maybe it's
> > not
> > > > even
> > > > > > > enforceable at all.
> > > > > > >     ________________________________________
> > > > > > >     From: Rawlin Peters <ra...@gmail.com>
> > > > > > >     Sent: Tuesday, October 23, 2018 11:14 AM
> > > > > > >     To: dev@trafficcontrol.apache.org
> > > > > > >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> > > > > > >
> > > > > > >     +1 on enabling squash and merge.
> > > > > > >
> > > > > > >     This will not only make cherry picks easier to manage with
> > > > respect to
> > > > > > >     merge conflicts, but it will also make the git log more
> > useful
> > > > because
> > > > > > >     we will be able to see the full changeset of the PR in a
> > single
> > > > commit
> > > > > > >     of the git log. Ideally, I'd also like the squashed commit
> to
> > > > contain
> > > > > > >     a link to the actual github PR so that I can easily view
> > > > comments, but
> > > > > > >     that is a proposal for another day.
> > > > > > >
> > > > > > >     - Rawlin
> > > > > > >
> > > > > > >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <
> > neuman@apache.org>
> > > > > > > wrote:
> > > > > > >     >
> > > > > > >     > This seems to have stalled, so let me try to move it
> along.
> > > > > > >     >
> > > > > > >     > There is an ability to do "squash and merge" in github,
> we
> > > > just need
> > > > > > > to
> > > > > > >     > agree that we want to enable it for our project.
> > > > > > >     >
> > > > > > >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure
> > > > exactly how
> > > > > > > to
> > > > > > >     > handle the multiple committers thing, but I assume we can
> > > > squash
> > > > > > > into two
> > > > > > >     > commits and give one to each.  However, if we are doing
> > > > something big
> > > > > > >     > enough to require that many commits from multiple users,
> it
> > > > may be
> > > > > > > best as
> > > > > > >     > many PRs?  I don't think this is a very common situation,
> > so I
> > > > would
> > > > > > > like
> > > > > > >     > to avoid getting bogged down in what-ifs if possible.
> > > > > > >     >
> > > > > > >     > As for the Changelog, I am fine with enabling squash and
> > merge
> > > > and
> > > > > > > keeping
> > > > > > >     > the changelog as-is while we decide if squash and merge
> > meets
> > > > our
> > > > > > > needs.  I
> > > > > > >     > am also game to revisit different ways to do it, and I
> > > > shouldn't have
> > > > > > >     > conflated that conversation with this one.  If someone
> > wants
> > > > to see
> > > > > > > that
> > > > > > >     > changed, we should start a new discussion.
> > > > > > >     >
> > > > > > >     > Leif brings up a good point about squash and merge making
> > > > cherry
> > > > > > > picks
> > > > > > >     > easier so I think we should consider it for that point
> > alone.
> > > > > > >     >
> > > > > > >     > So, what I am really asking is:  "do we think it is a
> good
> > > > idea to
> > > > > > > enable
> > > > > > >     > squash and merge on our Github repo so that we can reduce
> > the
> > > > amount
> > > > > > > of
> > > > > > >     > commits per PR?"  I would like to get consensus for or
> > > > against.  If
> > > > > > > it is
> > > > > > >     > not clear already, I am for.
> > > > > > >     >
> > > > > > >     >
> > > > > > >     > Thanks,
> > > > > > >     > Dave
> > > > > > >     >
> > > > > > >     >
> > > > > > >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > > > > > > Derek_Gelinas@comcast.com>
> > > > > > >     > wrote:
> > > > > > >     >
> > > > > > >     > > I'll be honest, I think the simplest thing is still to
> > just
> > > > > > > require a
> > > > > > >     > > changelog message, added to a central file, to be
> > included
> > > > in the
> > > > > > > commit.
> > > > > > >     > > We've been going over and over this for months instead
> of
> > > > actually
> > > > > > > writing
> > > > > > >     > > change logs.  Let's just propose it and put it to a
> vote.
> > > > > > >     > >
> > > > > > >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > > > > > > rawlin.peters@gmail.com>
> > > > > > >     > > wrote:
> > > > > > >     > > >
> > > > > > >     > > > I'm +1 on squashing and merging, but I think we need
> a
> > > > > > > combination of
> > > > > > >     > > > a number of things to really have a decent changelog.
> > Even
> > > > if we
> > > > > > > have
> > > > > > >     > > > awesome commit messages and squashed PRs, there's
> still
> > > > way too
> > > > > > > many
> > > > > > >     > > > PRs in a release to just generate a decent changelog
> > from
> > > > the PR
> > > > > > >     > > > titles and/or commit messages. We still need a way to
> > > > logically
> > > > > > >     > > > separate PRs into bugfixes, new features, breaking
> > > > changes, etc.
> > > > > > > We
> > > > > > >     > > > could get some of that separation using github tags,
> > but
> > > > what if
> > > > > > > there
> > > > > > >     > > > are multiple PRs that compose one new high-level
> > feature?
> > > > At that
> > > > > > >     > > > point I think we'd have to create a new tag for every
> > new
> > > > > > > high-level
> > > > > > >     > > > feature being worked on in order to group those PRs
> > under
> > > > the
> > > > > > > same
> > > > > > >     > > > heading. There's also the problem that only
> committers
> > can
> > > > tag
> > > > > > > issues,
> > > > > > >     > > > so it adds to the burden of committers to make sure
> all
> > > > > > > non-committer
> > > > > > >     > > > issues/PRs are properly tagged.
> > > > > > >     > > >
> > > > > > >     > > > The main headache with doing a manually-curated
> > changelog
> > > > is the
> > > > > > > merge
> > > > > > >     > > > conflicts it causes, but I think we can easily avoid
> > that
> > > > > > > problem by
> > > > > > >     > > > using a similar approach to reno [1] which breaks
> > > > individual
> > > > > > > notes out
> > > > > > >     > > > into their own separate yaml files which are then
> > parsed
> > > > and
> > > > > > > merged
> > > > > > >     > > > into a single, cohesive changelog for each release.
> > What
> > > > that
> > > > > > > means is
> > > > > > >     > > > basically every logical bugfix/new feature/upgrade
> note
> > > > gets its
> > > > > > > own
> > > > > > >     > > > file which avoids merge conflicts caused by just
> > adding a
> > > > line
> > > > > > > to the
> > > > > > >     > > > end of a CHANGELOG.md file every time.
> > > > > > >     > > >
> > > > > > >     > > > - Rawlin
> > > > > > >     > > >
> > > > > > >     > > > [1]
> > https://docs.openstack.org/reno/latest/user/usage.html
> > > > > > >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > > > > > > mitchell852@gmail.com>
> > > > > > >     > > wrote:
> > > > > > >     > > >>
> > > > > > >     > > >> is there a setting in github to enable "squash and
> > merge"
> > > > vs
> > > > > > > "rebase and
> > > > > > >     > > >> merge"?
> > > > > > >     > > >>
> > > > > > >     > > >> i do have a couple of concerns however:
> > > > > > >     > > >>
> > > > > > >     > > >> 1. what happens when 2+ people contribute to a PR?
> > This
> > > > is a
> > > > > > > pretty
> > > > > > >     > > common
> > > > > > >     > > >> scenario. If Bob and Sam both work on PR#1 is all
> the
> > > > commit
> > > > > > > history for
> > > > > > >     > > >> Sam lost?
> > > > > > >     > > >> 2. i can see benefits of having granular
> history...to
> > a
> > > > point...
> > > > > > >     > > >>
> > > > > > >     > > >> The more I think about this. Why is squash needed?
> > > > Commits are
> > > > > > > grouped
> > > > > > >     > > by
> > > > > > >     > > >> PRs. Why isn't our change log just a list of PR
> merged
> > > > since
> > > > > > > the last
> > > > > > >     > > >> release? If a PR represents a working unit (as it
> > > > should), PR
> > > > > > > titles
> > > > > > >     > > should
> > > > > > >     > > >> be sufficient, no?
> > > > > > >     > > >>
> > > > > > >     > > >> For example, this is what our internal release
> change
> > log
> > > > looks
> > > > > > > like.
> > > > > > >     > > >>
> > > > > > >     > > >> Changelog from 2526569a to 284555703
> > > > > > >     > > >> ========================================
> > > > > > >     > > >>
> > > > > > >     > > >> These changes are deployed in branch
> deploy-20181002.
> > > > > > >     > > >>
> > > > > > >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > > > > > >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add
> > TO Go
> > > > > > >     > > >> cachegroups/id/deliveryservices (Robert Butts, May
> 18
> > > > 2018)
> > > > > > >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add
> > TO Go
> > > > > > >     > > cachegroups/dses
> > > > > > >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > > > > > >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef:
> > Remove TO
> > > > > > > unused func
> > > > > > >     > > >> (Robert Butts, Sep 13 2018)
> > > > > > >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > > > > > >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add
> > TO Go
> > > > > > >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > > > > > >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add
> > TO
> > > > API
> > > > > > > Tests for
> > > > > > >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> > > > > > >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add
> > TO Go
> > > > > > > client
> > > > > > >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > > > > > >     > > >> PR 2806: Add sub to validate user input when running
> > > > > > > postinstall script
> > > > > > >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add
> > sub
> > > > to
> > > > > > > validate
> > > > > > >     > > user
> > > > > > >     > > >> input when running postinstall script changes
> default
> > cdn
> > > > name
> > > > > > > to not
> > > > > > >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > > > > > >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e:
> > Altering
> > > > > > > postinstall
> > > > > > >     > > cdn
> > > > > > >     > > >> name regex to include underscores (Dylan Souza, Sep
> 21
> > > > 2018)
> > > > > > >     > > >>        70d8893335b63db006974932341378717c42701c:
> > Changing
> > > > back
> > > > > > > the
> > > > > > >     > > default
> > > > > > >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> > > > > > >     > > >> PR 2812: remove traffic_monitor_java
> > > > > > >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de:
> > remove
> > > > > > >     > > traffic_monitor_java
> > > > > > >     > > >> (Dan Kirkwood, Sep 12 2018)
> > > > > > >     > > >>
> > > > > > >     > > >>
> > > > > > >     > > >> [truncated]
> > > > > > >     > > >>
> > > > > > >     > > >>
> > > > > > >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <
> > > > neuman@apache.org>
> > > > > > > wrote:
> > > > > > >     > > >>>
> > > > > > >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > > > > > > information on what
> > > > > > >     > > >>> actually went into the release, which therefore
> > makes it
> > > > not
> > > > > > > useful.
> > > > > > >     > > If we
> > > > > > >     > > >>> were better about commit messages and used squash
> and
> > > > merge,
> > > > > > > then we
> > > > > > >     > > would
> > > > > > >     > > >>> at least have something for every PR that was
> merged.
> > > > > > >     > > >>>
> > > > > > >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <
> > > > jvd@knutsel.com>
> > > > > > > wrote:
> > > > > > >     > > >>>
> > > > > > >     > > >>>>> I think this just shifts the burden from writing
> a
> > > > changelog
> > > > > > > entry to
> > > > > > >     > > >>>> writing a good commit entry.
> > > > > > >     > > >>>>
> > > > > > >     > > >>>>
> > > > > > >     > > >>>> I agree, and I think that’s where that burden
> > belongs.
> > > > (And I
> > > > > > > _really_
> > > > > > >     > > >>>> hate merge conflicts).
> > > > > > >     > > >>>>
> > > > > > >     > > >>>>> There might be fewer commit entries if we squash
> > and
> > > > merge,
> > > > > > > but I’m
> > > > > > >     > > >>>> doubtful that it would be as valuable as our
> > “curated”
> > > > > > > changelogs.
> > > > > > >     > > >>>>
> > > > > > >     > > >>>> Both are dependent on how disciplined we are.
> > > > > > >     > > >>>>
> > > > > > >     > > >>>> I’m +1 on Squash and Merge.
> > > > > > >     > > >>>>
> > > > > > >     > > >>>> Cheers,
> > > > > > >     > > >>>> JvD
> > > > > > >     > > >>>>
> > > > > > >     > > >>>>
> > > > > > >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich
> > (efriedri)
> > > > > > >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>> I think this just shifts the burden from writing
> a
> > > > changelog
> > > > > > > entry to
> > > > > > >     > > >>>> writing a good commit entry.
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>> There might be fewer commit entries if we squash
> > and
> > > > merge,
> > > > > > > but I’m
> > > > > > >     > > >>>> doubtful that it would be as valuable as our
> > “curated”
> > > > > > > changelogs.
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <
> > > > neuman@apache.org>
> > > > > > > wrote:
> > > > > > >     > > >>>>>>
> > > > > > >     > > >>>>>> Hey All,
> > > > > > >     > > >>>>>> I want to follow up on something that was
> briefly
> > > > discussed
> > > > > > > at the
> > > > > > >     > > >>>> summit
> > > > > > >     > > >>>>>> this week.  Many people do not like the
> Changelog
> > and
> > > > feel
> > > > > > > like it
> > > > > > >     > > can
> > > > > > >     > > >>>> be a
> > > > > > >     > > >>>>>> PITA to deal with.  One of the reasons we have
> the
> > > > > > > changelog is
> > > > > > >     > > >>> because
> > > > > > >     > > >>>>>> there are so many commits in a given release,
> > that it
> > > > is
> > > > > > > hard to
> > > > > > >     > > comb
> > > > > > >     > > >>>>>> through all of them to figure out what
> noteworthy
> > > > changes
> > > > > > > or bug
> > > > > > >     > > fixes
> > > > > > >     > > >>>> went
> > > > > > >     > > >>>>>> into the code.  One thing that may help with
> this
> > > > problem
> > > > > > > is to use
> > > > > > >     > > >>>> enable
> > > > > > >     > > >>>>>> the squash and merge feature on Github for pull
> > > > requests
> > > > > > > [1].   This
> > > > > > >     > > >>>>>> feature would squash all commits in a PR into
> one
> > > > commit.
> > > > > > > If we
> > > > > > >     > > pair
> > > > > > >     > > >>>> the
> > > > > > >     > > >>>>>> one commit with a good commit message, we would
> > > > essentially
> > > > > > > get the
> > > > > > >     > > >>>> ability
> > > > > > >     > > >>>>>> to create the changelog just from the commits.
> > > > > > >     > > >>>>>>
> > > > > > >     > > >>>>>> So, I would like to propose that we enable the
> > squash
> > > > and
> > > > > > > merge
> > > > > > >     > > >>> feature
> > > > > > >     > > >>>> in
> > > > > > >     > > >>>>>> the Traffic Control Github repo and use that
> going
> > > > forward.
> > > > > > >     > > Thoughts?
> > > > > > >     > > >>>>>>
> > > > > > >     > > >>>>>> Thanks,
> > > > > > >     > > >>>>>> Dave
> > > > > > >     > > >>>>>>
> > > > > > >     > > >>>>>>
> > > > > > >     > > >>>>>> [1]
> > > > > > > https://help.github.com/articles/about-pull-request-merges/
> > > > > > >     > > >>>>>
> > > > > > >     > > >>>>
> > > > > > >     > > >>>>
> > > > > > >     > > >>>
> > > > > > >     > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > >
> >
>

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Jeremy Mitchell <mi...@gmail.com>]

After reading Chris' quite detailed response :) I'm leaning towards `Squash
and Merge` but the PR needs to be as atomic as possible. And we should
expect committers to enforce this. I.e.

some people have PR's that look like this:

PR title: Adds Feature X
- commit 1 - cranking out some API endpoints
- commit 2 - writing some API tests
- commit 3 - cranking out some UI code
- commit 4 - writing some UI tests
- commit 5 - Adding some database tables
- commit 6 - writing some documentation for feature X

some people have PR's that look like this:

PR title: Adds Feature X
- commit 1 - Adds Feature X

regardless of the approach, squash that down into one commit with message
of `Adds Feature X`

Hopefully, that will result in smaller PR's that can get merged faster.

+1 squash



On Wed, Oct 24, 2018 at 5:12 PM Chris Lemmons <al...@gmail.com> wrote:

> Right, the primary difference between "squash and merge" and "rebase
> and merge" is who is responsible for writing the commit message.
> Squash asks the committer to do it, as part of the squash. Rebase asks
> the author to do it, as part of the commits in the PR. Generally
> speaking, I think authors should be responsible for their own commit
> messages. It can be a bit tough to find committers for merging PRs as
> it is, I don't know that asking folks to write commit logs for
> everything they merge is likely to improve things. :/ In my mind, a
> really solid PR process example might look like this:
>
> 1. Author implements a moderately sized feature.
> 2. The author creates commits, one for each major atomic unit of code
> for the feature (usually, just one commit). The commit messages have
> subjects and bodies, like I mentioned before. Each commit has all the
> relevant testing and documentation that is associated with the code it
> contains.
> 3. The author creates a PR. The subject of the PR is the broad feature
> title. The body contains the information we use the template to ask
> for.
> 4. A committer comes by and identifies areas for improvement. (A
> non-committer may also join the review, but at least one committer is
> involved, naturally.)
> 5. The author improves those areas until everyone reaches a consensus
> on the mergeability.
> 6. If there were improvement commits, the author does a `rebase -i` to
> squash those improvement commits appropriately up into the originals,
> leaving the same set of commits the PR started with, but better than
> before, because of the review.
> 7. If, during this time, master has drifted so that the branch no
> longer rebases cleanly, the author also rebases onto master and fixes
> the conflicts.
> 8. The author force-pushes to the branch.
> 9. A committer does one last sanity check to make sure the commits
> look right and hits "rebase and merge".
>
> For comparison, here's an example of what I think a squash process
> might look like:
>
> 1. Author implements a moderately sized feature.
> 2. The author creates commits as they go, with mildly useful, but not
> really informative commit messages.
> 3. The author creates a PR. The subject and body contain the
> information one might expect in the subject and body of an atomic
> commit.
> 4. A committer comes by and identifies areas for improvement. (And
> non-committers, as appropriate.)
> 5. The author adds commits to the PR to address things until consensus
> is reached.
> 6. If master has drifted, the author rebases onto master and force-pushes.
> 7. The committer hits "squash and merge" and, using the PR title and
> body as a guide, possibly verbatim.
>
> The squash and merge require slightly more of the committer and
> slightly less of the author, and it doesn't work if the unit of work
> isn't a single atomic unit of code. But for most of our work, I think
> it amounts to the same thing.
>
> And then, we can use `git log` to produce changelogs. If we really
> want to, we could use the standardized words from Keep a Changelog to
> identify kinds of commits:
>
> > "Added" for new features.
> > "Changed" for changes in existing functionality.
> > "Deprecated" for soon-to-be removed features.
> > "Removed" for now removed features.
> > "Fixed" for any bug fixes.
>
> Ultimately, PRs are transitory, so I'd really like for any genuinely
> important information to make its way into a commit message that will
> stick around for a while.
> On Wed, Oct 24, 2018 at 10:44 AM Jeremy Mitchell <mi...@gmail.com>
> wrote:
> >
> > I do like the idea of adding the 'squash and merge` option so we end up
> > with 2 Github options:
> >
> > - rebase and merge
> > - squash and merge
> >
> > Leave it up to the merger which option to choose.
> >
> > Now as far as how to create a "change log", back to what Leif said
> earlier:
> >
> > "Why not generate the ChangeLog from the actual PR “subject” line? Then
> it
> > wouldn’t matter if you have 1 or 100 commits for a particular PR. The
> > Sorber wrote a little script for ATS that generates our ChangeLogs *)"
> >
> >
> >
> > On Wed, Oct 24, 2018 at 10:24 AM Rawlin Peters <ra...@gmail.com>
> > wrote:
> >
> > > I agree with most of this and wish everyone would structure their PRs
> > > and commits like this, but the fact of the matter is that small
> > > checkpoint commits are simply just the way some people like to work.
> > > For the last few things I've worked on, I've tried to separate changes
> > > to the different components for the same logical feature into
> > > different PRs (API, UI, Traffic Router). I find that breaking down the
> > > feature into a PR per component makes things a bit easier to review,
> > > since not everyone has experience all the various TC components that
> > > could be mashed into one mega-PR. If a mega-PR contains changes to TO,
> > > TP, and TR all in one, some people maybe be comfortable reviewing TO
> > > but not necessarily TP or TR, so you end up having to coordinate with
> > > multiple reviewers/committers who are comfortable reviewing different
> > > areas just to get a single PR merged.
> > >
> > > Unless we start making good commit messages and "one standalone
> > > change" per commit a hard requirement for all contributors to the
> > > project, I doubt we can ever get to this ideal, and I'm not sure we
> > > should make that a requirement right now. However, by squashing and
> > > merging commits, we can clean up the git history by removing all of
> > > the non-atomic/checkpoint commits as well as other benefits that have
> > > already been discussed.
> > >
> > > Nonetheless, we will still be able to use the "rebase and merge"
> > > option at the committer's discretion. If the contributor and committer
> > > feel that the commits in the PR are more valuable in the history
> > > because they're atomic and each have good commit messages, the
> > > committer should choose the "rebase and merge" option. If the PR
> > > contains lots of small, nonatomic, checkpoint commits that would
> > > obfuscate the git history and make it harder for someone doing a `git
> > > blame` to figure out why a change was made, then maybe it would be
> > > better for the committer to choose the "squash and merge" option or
> > > kindly ask the PR submitter to squash the commits themselves into a
> > > single commit and write a good title+message.
> > >
> > > - Rawlin
> > >
> > > On Tue, Oct 23, 2018 at 4:49 PM Chris Lemmons <al...@gmail.com>
> wrote:
> > > >
> > > > I don't think I'm quite -1 on this, but I think we can do better.
> > > >
> > > > I think a PR should be a single unit of work and a commit should be a
> > > > single unit of code.
> > > >
> > > > I like using atomic commits. That is, each commit does one thing. So
> > > > if I need to revert a fix or track down when something was
> introduced,
> > > > I can find it. For commit messages, I really like the seven rules
> > > > documented here, but especially the last one:
> > > > https://chris.beams.io/posts/git-commit/
> > > >
> > > > > Use the body to explain what and why vs. how
> > > >
> > > > I think it's important to put the body in the commit message in order
> > > > to explain why the change is necessary what the non-obvious effects
> of
> > > > it are. Having a good subject for the commit is also very useful,
> > > > since that's what GitHub (and really all the tools) shows when you
> > > > scroll through commits, looking for what changed.
> > > >
> > > > In order to have atomic commits, sometimes you'll need one or more
> > > > commits as part of a single PR. Not most of the time, but sometimes.
> > > > Especially with a large feature, you may make multiple dependent
> > > > changes, but each one is a logical unit of work. For example, if the
> > > > task is "add support for foobaz plugin", there might be three
> commits,
> > > > with summaries that look like "Add support for foobaz plugin to
> config
> > > > generation", "Add UI for editing foobaz plugin config", and "Add API
> > > > endpoints for foobaz plugin config". That also makes it easy to
> direct
> > > > domain experts to each of the changesets for focused review.
> > > >
> > > > A PR on the other hand, is a single unit of work. While each commit
> > > > should be stand-alone, capable of running correctly before and after,
> > > > each commit might not be desirable on a standalone basis. For
> example,
> > > > while "Add support for foobaz plugin to config generation" is a
> single
> > > > unit of code in that it operates correctly, has tests, is documented,
> > > > and is completely implemented, it's not exceptionally useful without
> a
> > > > UI to edit it. It makes sense to group all these changes and merge
> > > > them as a unit.
> > > >
> > > > For almost all bug fixes and most medium to small features, each PR
> > > > will have precisely one commit. But especially for larger "projects",
> > > > it makes the most sense to group it all as one PR, but merge it as a
> > > > feature.
> > > >
> > > > Relatedly, it's very rarely correct to merge commits from two
> > > > different authors. (This is important for legal reasons as well, if
> it
> > > > ever came to it; authors license their code when they contribute it,
> > > > so it's important to know whence it came.) However, in the event that
> > > > two people jointly produce what amounts to a single unit of code (for
> > > > example, by pair programming, or by collaborating on a snippet of
> code
> > > > on Slack or IRC), you can document that in git by adding it to the
> end
> > > > of the commit message body:
> > > >
> > > > > Co-authored-by: name <na...@example.com>
> > > > > Co-authored-by: another-name <an...@example.com>"
> > > >
> > > > Details for that technique here:
> > > >
> > >
> https://help.github.com/articles/creating-a-commit-with-multiple-authors/
> > > >
> > > > It's not quite a "standard", but it's quickly being adopted by most
> of
> > > > the tools I use, at least.
> > > >
> > > > As for the "Squash" option, I actually prefer the "Rebase" choice.
> > > > It's the same thing if you have single-commit PRs, but it handles
> > > > multi-commit PRs a bit more nicely.
> > > >
> > > > So, to summarize, here's what I think:
> > > >
> > > >  - Each commit should be an atomic unit of code and be described by a
> > > > descriptive commit subject and body.
> > > >  - Each PR should be a logical grouping of closely related work;
> > > > usually a single commit.
> > > >  - We should use Rebase for flattening our changes.
> > > >  - Squash is still an "okay" heuristic that accomplishes the same
> > > > thing, most of the time.
> > > > On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <
> dewrich@apache.org>
> > > wrote:
> > > > >
> > > > > +1 based upon the fact that the Changelog.md can't seem to stay
> > > current, we
> > > > > do need to be able to maybe at least generate one from the commit
> > > messages
> > > > > within that release window.
> > > > >
> > > > > -Dew
> > > > >
> > > > > On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <
> > > Jonathan_Gray@comcast.com>
> > > > > wrote:
> > > > >
> > > > > > +1
> > > > > >
> > > > > >  In a past life when using Microsoft VSTS with git, the default
> > > action was
> > > > > > squash.  The rule of thumb we used was squash by default and only
> > > merge if
> > > > > > every commit was meaningful and required to understand what you
> were
> > > trying
> > > > > > to do.  We didn't typically have communal feature branches, so it
> > > wasn't as
> > > > > > large a use case for us.  The repo forensics are nice with merge,
> > > but the
> > > > > > reality we found was that git blame was more useful when we could
> > > track to
> > > > > > the PR rather than a mid-stream commit and then having to make a
> > > second
> > > > > > jump to the PR from there.  It also cut down on back/forth design
> > > change
> > > > > > commits and helped us stay on track with 1 PR == 1 issue because
> it
> > > made
> > > > > > unrelated fixes more apparent.
> > > > > >
> > > > > > Jonathan G
> > > > > >
> > > > > >
> > > > > > On 10/23/18, 12:40 PM, "Fieck, Brennan" <
> Brennan_Fieck@comcast.com>
> > > > > > wrote:
> > > > > >
> > > > > >     I'm not gonna say +1 or -1, but I'd still like to chime in. I
> > > think
> > > > > > it'd be nice to require PRs to squash things into atomic chunks.
> No
> > > commits
> > > > > > that say "started x" or "minor fixes", but idk if it's
> reasonable to
> > > be
> > > > > > able to boil every PR forever down to 70 characters. And sure you
> > > can make
> > > > > > multiple PRs if changes are too big to fit in one commit message,
> > > but then
> > > > > > it becomes really easy to become entwined in a chain of dependent
> > > PRs so
> > > > > > that every time one of them gets merged you have to rebase them
> all.
> > > > > >
> > > > > >     Of course, a "reasonable squash" is pretty hard to define
> > > absolutely,
> > > > > > and could actually just make PRs harder to review -  maybe it's
> not
> > > even
> > > > > > enforceable at all.
> > > > > >     ________________________________________
> > > > > >     From: Rawlin Peters <ra...@gmail.com>
> > > > > >     Sent: Tuesday, October 23, 2018 11:14 AM
> > > > > >     To: dev@trafficcontrol.apache.org
> > > > > >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> > > > > >
> > > > > >     +1 on enabling squash and merge.
> > > > > >
> > > > > >     This will not only make cherry picks easier to manage with
> > > respect to
> > > > > >     merge conflicts, but it will also make the git log more
> useful
> > > because
> > > > > >     we will be able to see the full changeset of the PR in a
> single
> > > commit
> > > > > >     of the git log. Ideally, I'd also like the squashed commit to
> > > contain
> > > > > >     a link to the actual github PR so that I can easily view
> > > comments, but
> > > > > >     that is a proposal for another day.
> > > > > >
> > > > > >     - Rawlin
> > > > > >
> > > > > >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <
> neuman@apache.org>
> > > > > > wrote:
> > > > > >     >
> > > > > >     > This seems to have stalled, so let me try to move it along.
> > > > > >     >
> > > > > >     > There is an ability to do "squash and merge" in github, we
> > > just need
> > > > > > to
> > > > > >     > agree that we want to enable it for our project.
> > > > > >     >
> > > > > >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure
> > > exactly how
> > > > > > to
> > > > > >     > handle the multiple committers thing, but I assume we can
> > > squash
> > > > > > into two
> > > > > >     > commits and give one to each.  However, if we are doing
> > > something big
> > > > > >     > enough to require that many commits from multiple users, it
> > > may be
> > > > > > best as
> > > > > >     > many PRs?  I don't think this is a very common situation,
> so I
> > > would
> > > > > > like
> > > > > >     > to avoid getting bogged down in what-ifs if possible.
> > > > > >     >
> > > > > >     > As for the Changelog, I am fine with enabling squash and
> merge
> > > and
> > > > > > keeping
> > > > > >     > the changelog as-is while we decide if squash and merge
> meets
> > > our
> > > > > > needs.  I
> > > > > >     > am also game to revisit different ways to do it, and I
> > > shouldn't have
> > > > > >     > conflated that conversation with this one.  If someone
> wants
> > > to see
> > > > > > that
> > > > > >     > changed, we should start a new discussion.
> > > > > >     >
> > > > > >     > Leif brings up a good point about squash and merge making
> > > cherry
> > > > > > picks
> > > > > >     > easier so I think we should consider it for that point
> alone.
> > > > > >     >
> > > > > >     > So, what I am really asking is:  "do we think it is a good
> > > idea to
> > > > > > enable
> > > > > >     > squash and merge on our Github repo so that we can reduce
> the
> > > amount
> > > > > > of
> > > > > >     > commits per PR?"  I would like to get consensus for or
> > > against.  If
> > > > > > it is
> > > > > >     > not clear already, I am for.
> > > > > >     >
> > > > > >     >
> > > > > >     > Thanks,
> > > > > >     > Dave
> > > > > >     >
> > > > > >     >
> > > > > >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > > > > > Derek_Gelinas@comcast.com>
> > > > > >     > wrote:
> > > > > >     >
> > > > > >     > > I'll be honest, I think the simplest thing is still to
> just
> > > > > > require a
> > > > > >     > > changelog message, added to a central file, to be
> included
> > > in the
> > > > > > commit.
> > > > > >     > > We've been going over and over this for months instead of
> > > actually
> > > > > > writing
> > > > > >     > > change logs.  Let's just propose it and put it to a vote.
> > > > > >     > >
> > > > > >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > > > > > rawlin.peters@gmail.com>
> > > > > >     > > wrote:
> > > > > >     > > >
> > > > > >     > > > I'm +1 on squashing and merging, but I think we need a
> > > > > > combination of
> > > > > >     > > > a number of things to really have a decent changelog.
> Even
> > > if we
> > > > > > have
> > > > > >     > > > awesome commit messages and squashed PRs, there's still
> > > way too
> > > > > > many
> > > > > >     > > > PRs in a release to just generate a decent changelog
> from
> > > the PR
> > > > > >     > > > titles and/or commit messages. We still need a way to
> > > logically
> > > > > >     > > > separate PRs into bugfixes, new features, breaking
> > > changes, etc.
> > > > > > We
> > > > > >     > > > could get some of that separation using github tags,
> but
> > > what if
> > > > > > there
> > > > > >     > > > are multiple PRs that compose one new high-level
> feature?
> > > At that
> > > > > >     > > > point I think we'd have to create a new tag for every
> new
> > > > > > high-level
> > > > > >     > > > feature being worked on in order to group those PRs
> under
> > > the
> > > > > > same
> > > > > >     > > > heading. There's also the problem that only committers
> can
> > > tag
> > > > > > issues,
> > > > > >     > > > so it adds to the burden of committers to make sure all
> > > > > > non-committer
> > > > > >     > > > issues/PRs are properly tagged.
> > > > > >     > > >
> > > > > >     > > > The main headache with doing a manually-curated
> changelog
> > > is the
> > > > > > merge
> > > > > >     > > > conflicts it causes, but I think we can easily avoid
> that
> > > > > > problem by
> > > > > >     > > > using a similar approach to reno [1] which breaks
> > > individual
> > > > > > notes out
> > > > > >     > > > into their own separate yaml files which are then
> parsed
> > > and
> > > > > > merged
> > > > > >     > > > into a single, cohesive changelog for each release.
> What
> > > that
> > > > > > means is
> > > > > >     > > > basically every logical bugfix/new feature/upgrade note
> > > gets its
> > > > > > own
> > > > > >     > > > file which avoids merge conflicts caused by just
> adding a
> > > line
> > > > > > to the
> > > > > >     > > > end of a CHANGELOG.md file every time.
> > > > > >     > > >
> > > > > >     > > > - Rawlin
> > > > > >     > > >
> > > > > >     > > > [1]
> https://docs.openstack.org/reno/latest/user/usage.html
> > > > > >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > > > > > mitchell852@gmail.com>
> > > > > >     > > wrote:
> > > > > >     > > >>
> > > > > >     > > >> is there a setting in github to enable "squash and
> merge"
> > > vs
> > > > > > "rebase and
> > > > > >     > > >> merge"?
> > > > > >     > > >>
> > > > > >     > > >> i do have a couple of concerns however:
> > > > > >     > > >>
> > > > > >     > > >> 1. what happens when 2+ people contribute to a PR?
> This
> > > is a
> > > > > > pretty
> > > > > >     > > common
> > > > > >     > > >> scenario. If Bob and Sam both work on PR#1 is all the
> > > commit
> > > > > > history for
> > > > > >     > > >> Sam lost?
> > > > > >     > > >> 2. i can see benefits of having granular history...to
> a
> > > point...
> > > > > >     > > >>
> > > > > >     > > >> The more I think about this. Why is squash needed?
> > > Commits are
> > > > > > grouped
> > > > > >     > > by
> > > > > >     > > >> PRs. Why isn't our change log just a list of PR merged
> > > since
> > > > > > the last
> > > > > >     > > >> release? If a PR represents a working unit (as it
> > > should), PR
> > > > > > titles
> > > > > >     > > should
> > > > > >     > > >> be sufficient, no?
> > > > > >     > > >>
> > > > > >     > > >> For example, this is what our internal release change
> log
> > > looks
> > > > > > like.
> > > > > >     > > >>
> > > > > >     > > >> Changelog from 2526569a to 284555703
> > > > > >     > > >> ========================================
> > > > > >     > > >>
> > > > > >     > > >> These changes are deployed in branch deploy-20181002.
> > > > > >     > > >>
> > > > > >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > > > > >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add
> TO Go
> > > > > >     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18
> > > 2018)
> > > > > >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add
> TO Go
> > > > > >     > > cachegroups/dses
> > > > > >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > > > > >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef:
> Remove TO
> > > > > > unused func
> > > > > >     > > >> (Robert Butts, Sep 13 2018)
> > > > > >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > > > > >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add
> TO Go
> > > > > >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > > > > >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add
> TO
> > > API
> > > > > > Tests for
> > > > > >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> > > > > >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add
> TO Go
> > > > > > client
> > > > > >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > > > > >     > > >> PR 2806: Add sub to validate user input when running
> > > > > > postinstall script
> > > > > >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add
> sub
> > > to
> > > > > > validate
> > > > > >     > > user
> > > > > >     > > >> input when running postinstall script changes default
> cdn
> > > name
> > > > > > to not
> > > > > >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > > > > >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e:
> Altering
> > > > > > postinstall
> > > > > >     > > cdn
> > > > > >     > > >> name regex to include underscores (Dylan Souza, Sep 21
> > > 2018)
> > > > > >     > > >>        70d8893335b63db006974932341378717c42701c:
> Changing
> > > back
> > > > > > the
> > > > > >     > > default
> > > > > >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> > > > > >     > > >> PR 2812: remove traffic_monitor_java
> > > > > >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de:
> remove
> > > > > >     > > traffic_monitor_java
> > > > > >     > > >> (Dan Kirkwood, Sep 12 2018)
> > > > > >     > > >>
> > > > > >     > > >>
> > > > > >     > > >> [truncated]
> > > > > >     > > >>
> > > > > >     > > >>
> > > > > >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <
> > > neuman@apache.org>
> > > > > > wrote:
> > > > > >     > > >>>
> > > > > >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > > > > > information on what
> > > > > >     > > >>> actually went into the release, which therefore
> makes it
> > > not
> > > > > > useful.
> > > > > >     > > If we
> > > > > >     > > >>> were better about commit messages and used squash and
> > > merge,
> > > > > > then we
> > > > > >     > > would
> > > > > >     > > >>> at least have something for every PR that was merged.
> > > > > >     > > >>>
> > > > > >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <
> > > jvd@knutsel.com>
> > > > > > wrote:
> > > > > >     > > >>>
> > > > > >     > > >>>>> I think this just shifts the burden from writing a
> > > changelog
> > > > > > entry to
> > > > > >     > > >>>> writing a good commit entry.
> > > > > >     > > >>>>
> > > > > >     > > >>>>
> > > > > >     > > >>>> I agree, and I think that’s where that burden
> belongs.
> > > (And I
> > > > > > _really_
> > > > > >     > > >>>> hate merge conflicts).
> > > > > >     > > >>>>
> > > > > >     > > >>>>> There might be fewer commit entries if we squash
> and
> > > merge,
> > > > > > but I’m
> > > > > >     > > >>>> doubtful that it would be as valuable as our
> “curated”
> > > > > > changelogs.
> > > > > >     > > >>>>
> > > > > >     > > >>>> Both are dependent on how disciplined we are.
> > > > > >     > > >>>>
> > > > > >     > > >>>> I’m +1 on Squash and Merge.
> > > > > >     > > >>>>
> > > > > >     > > >>>> Cheers,
> > > > > >     > > >>>> JvD
> > > > > >     > > >>>>
> > > > > >     > > >>>>
> > > > > >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich
> (efriedri)
> > > > > >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> > > > > >     > > >>>>>
> > > > > >     > > >>>>> I think this just shifts the burden from writing a
> > > changelog
> > > > > > entry to
> > > > > >     > > >>>> writing a good commit entry.
> > > > > >     > > >>>>>
> > > > > >     > > >>>>> There might be fewer commit entries if we squash
> and
> > > merge,
> > > > > > but I’m
> > > > > >     > > >>>> doubtful that it would be as valuable as our
> “curated”
> > > > > > changelogs.
> > > > > >     > > >>>>>
> > > > > >     > > >>>>>
> > > > > >     > > >>>>>
> > > > > >     > > >>>>>
> > > > > >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <
> > > neuman@apache.org>
> > > > > > wrote:
> > > > > >     > > >>>>>>
> > > > > >     > > >>>>>> Hey All,
> > > > > >     > > >>>>>> I want to follow up on something that was briefly
> > > discussed
> > > > > > at the
> > > > > >     > > >>>> summit
> > > > > >     > > >>>>>> this week.  Many people do not like the Changelog
> and
> > > feel
> > > > > > like it
> > > > > >     > > can
> > > > > >     > > >>>> be a
> > > > > >     > > >>>>>> PITA to deal with.  One of the reasons we have the
> > > > > > changelog is
> > > > > >     > > >>> because
> > > > > >     > > >>>>>> there are so many commits in a given release,
> that it
> > > is
> > > > > > hard to
> > > > > >     > > comb
> > > > > >     > > >>>>>> through all of them to figure out what noteworthy
> > > changes
> > > > > > or bug
> > > > > >     > > fixes
> > > > > >     > > >>>> went
> > > > > >     > > >>>>>> into the code.  One thing that may help with this
> > > problem
> > > > > > is to use
> > > > > >     > > >>>> enable
> > > > > >     > > >>>>>> the squash and merge feature on Github for pull
> > > requests
> > > > > > [1].   This
> > > > > >     > > >>>>>> feature would squash all commits in a PR into one
> > > commit.
> > > > > > If we
> > > > > >     > > pair
> > > > > >     > > >>>> the
> > > > > >     > > >>>>>> one commit with a good commit message, we would
> > > essentially
> > > > > > get the
> > > > > >     > > >>>> ability
> > > > > >     > > >>>>>> to create the changelog just from the commits.
> > > > > >     > > >>>>>>
> > > > > >     > > >>>>>> So, I would like to propose that we enable the
> squash
> > > and
> > > > > > merge
> > > > > >     > > >>> feature
> > > > > >     > > >>>> in
> > > > > >     > > >>>>>> the Traffic Control Github repo and use that going
> > > forward.
> > > > > >     > > Thoughts?
> > > > > >     > > >>>>>>
> > > > > >     > > >>>>>> Thanks,
> > > > > >     > > >>>>>> Dave
> > > > > >     > > >>>>>>
> > > > > >     > > >>>>>>
> > > > > >     > > >>>>>> [1]
> > > > > > https://help.github.com/articles/about-pull-request-merges/
> > > > > >     > > >>>>>
> > > > > >     > > >>>>
> > > > > >     > > >>>>
> > > > > >     > > >>>
> > > > > >     > >
> > > > > >
> > > > > >
> > > > > >
> > >
>

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Chris Lemmons <al...@gmail.com>]

Right, the primary difference between "squash and merge" and "rebase
and merge" is who is responsible for writing the commit message.
Squash asks the committer to do it, as part of the squash. Rebase asks
the author to do it, as part of the commits in the PR. Generally
speaking, I think authors should be responsible for their own commit
messages. It can be a bit tough to find committers for merging PRs as
it is, I don't know that asking folks to write commit logs for
everything they merge is likely to improve things. :/ In my mind, a
really solid PR process example might look like this:

1. Author implements a moderately sized feature.
2. The author creates commits, one for each major atomic unit of code
for the feature (usually, just one commit). The commit messages have
subjects and bodies, like I mentioned before. Each commit has all the
relevant testing and documentation that is associated with the code it
contains.
3. The author creates a PR. The subject of the PR is the broad feature
title. The body contains the information we use the template to ask
for.
4. A committer comes by and identifies areas for improvement. (A
non-committer may also join the review, but at least one committer is
involved, naturally.)
5. The author improves those areas until everyone reaches a consensus
on the mergeability.
6. If there were improvement commits, the author does a `rebase -i` to
squash those improvement commits appropriately up into the originals,
leaving the same set of commits the PR started with, but better than
before, because of the review.
7. If, during this time, master has drifted so that the branch no
longer rebases cleanly, the author also rebases onto master and fixes
the conflicts.
8. The author force-pushes to the branch.
9. A committer does one last sanity check to make sure the commits
look right and hits "rebase and merge".

For comparison, here's an example of what I think a squash process
might look like:

1. Author implements a moderately sized feature.
2. The author creates commits as they go, with mildly useful, but not
really informative commit messages.
3. The author creates a PR. The subject and body contain the
information one might expect in the subject and body of an atomic
commit.
4. A committer comes by and identifies areas for improvement. (And
non-committers, as appropriate.)
5. The author adds commits to the PR to address things until consensus
is reached.
6. If master has drifted, the author rebases onto master and force-pushes.
7. The committer hits "squash and merge" and, using the PR title and
body as a guide, possibly verbatim.

The squash and merge require slightly more of the committer and
slightly less of the author, and it doesn't work if the unit of work
isn't a single atomic unit of code. But for most of our work, I think
it amounts to the same thing.

And then, we can use `git log` to produce changelogs. If we really
want to, we could use the standardized words from Keep a Changelog to
identify kinds of commits:

> "Added" for new features.
> "Changed" for changes in existing functionality.
> "Deprecated" for soon-to-be removed features.
> "Removed" for now removed features.
> "Fixed" for any bug fixes.

Ultimately, PRs are transitory, so I'd really like for any genuinely
important information to make its way into a commit message that will
stick around for a while.
On Wed, Oct 24, 2018 at 10:44 AM Jeremy Mitchell <mi...@gmail.com> wrote:
>
> I do like the idea of adding the 'squash and merge` option so we end up
> with 2 Github options:
>
> - rebase and merge
> - squash and merge
>
> Leave it up to the merger which option to choose.
>
> Now as far as how to create a "change log", back to what Leif said earlier:
>
> "Why not generate the ChangeLog from the actual PR “subject” line? Then it
> wouldn’t matter if you have 1 or 100 commits for a particular PR. The
> Sorber wrote a little script for ATS that generates our ChangeLogs *)"
>
>
>
> On Wed, Oct 24, 2018 at 10:24 AM Rawlin Peters <ra...@gmail.com>
> wrote:
>
> > I agree with most of this and wish everyone would structure their PRs
> > and commits like this, but the fact of the matter is that small
> > checkpoint commits are simply just the way some people like to work.
> > For the last few things I've worked on, I've tried to separate changes
> > to the different components for the same logical feature into
> > different PRs (API, UI, Traffic Router). I find that breaking down the
> > feature into a PR per component makes things a bit easier to review,
> > since not everyone has experience all the various TC components that
> > could be mashed into one mega-PR. If a mega-PR contains changes to TO,
> > TP, and TR all in one, some people maybe be comfortable reviewing TO
> > but not necessarily TP or TR, so you end up having to coordinate with
> > multiple reviewers/committers who are comfortable reviewing different
> > areas just to get a single PR merged.
> >
> > Unless we start making good commit messages and "one standalone
> > change" per commit a hard requirement for all contributors to the
> > project, I doubt we can ever get to this ideal, and I'm not sure we
> > should make that a requirement right now. However, by squashing and
> > merging commits, we can clean up the git history by removing all of
> > the non-atomic/checkpoint commits as well as other benefits that have
> > already been discussed.
> >
> > Nonetheless, we will still be able to use the "rebase and merge"
> > option at the committer's discretion. If the contributor and committer
> > feel that the commits in the PR are more valuable in the history
> > because they're atomic and each have good commit messages, the
> > committer should choose the "rebase and merge" option. If the PR
> > contains lots of small, nonatomic, checkpoint commits that would
> > obfuscate the git history and make it harder for someone doing a `git
> > blame` to figure out why a change was made, then maybe it would be
> > better for the committer to choose the "squash and merge" option or
> > kindly ask the PR submitter to squash the commits themselves into a
> > single commit and write a good title+message.
> >
> > - Rawlin
> >
> > On Tue, Oct 23, 2018 at 4:49 PM Chris Lemmons <al...@gmail.com> wrote:
> > >
> > > I don't think I'm quite -1 on this, but I think we can do better.
> > >
> > > I think a PR should be a single unit of work and a commit should be a
> > > single unit of code.
> > >
> > > I like using atomic commits. That is, each commit does one thing. So
> > > if I need to revert a fix or track down when something was introduced,
> > > I can find it. For commit messages, I really like the seven rules
> > > documented here, but especially the last one:
> > > https://chris.beams.io/posts/git-commit/
> > >
> > > > Use the body to explain what and why vs. how
> > >
> > > I think it's important to put the body in the commit message in order
> > > to explain why the change is necessary what the non-obvious effects of
> > > it are. Having a good subject for the commit is also very useful,
> > > since that's what GitHub (and really all the tools) shows when you
> > > scroll through commits, looking for what changed.
> > >
> > > In order to have atomic commits, sometimes you'll need one or more
> > > commits as part of a single PR. Not most of the time, but sometimes.
> > > Especially with a large feature, you may make multiple dependent
> > > changes, but each one is a logical unit of work. For example, if the
> > > task is "add support for foobaz plugin", there might be three commits,
> > > with summaries that look like "Add support for foobaz plugin to config
> > > generation", "Add UI for editing foobaz plugin config", and "Add API
> > > endpoints for foobaz plugin config". That also makes it easy to direct
> > > domain experts to each of the changesets for focused review.
> > >
> > > A PR on the other hand, is a single unit of work. While each commit
> > > should be stand-alone, capable of running correctly before and after,
> > > each commit might not be desirable on a standalone basis. For example,
> > > while "Add support for foobaz plugin to config generation" is a single
> > > unit of code in that it operates correctly, has tests, is documented,
> > > and is completely implemented, it's not exceptionally useful without a
> > > UI to edit it. It makes sense to group all these changes and merge
> > > them as a unit.
> > >
> > > For almost all bug fixes and most medium to small features, each PR
> > > will have precisely one commit. But especially for larger "projects",
> > > it makes the most sense to group it all as one PR, but merge it as a
> > > feature.
> > >
> > > Relatedly, it's very rarely correct to merge commits from two
> > > different authors. (This is important for legal reasons as well, if it
> > > ever came to it; authors license their code when they contribute it,
> > > so it's important to know whence it came.) However, in the event that
> > > two people jointly produce what amounts to a single unit of code (for
> > > example, by pair programming, or by collaborating on a snippet of code
> > > on Slack or IRC), you can document that in git by adding it to the end
> > > of the commit message body:
> > >
> > > > Co-authored-by: name <na...@example.com>
> > > > Co-authored-by: another-name <an...@example.com>"
> > >
> > > Details for that technique here:
> > >
> > https://help.github.com/articles/creating-a-commit-with-multiple-authors/
> > >
> > > It's not quite a "standard", but it's quickly being adopted by most of
> > > the tools I use, at least.
> > >
> > > As for the "Squash" option, I actually prefer the "Rebase" choice.
> > > It's the same thing if you have single-commit PRs, but it handles
> > > multi-commit PRs a bit more nicely.
> > >
> > > So, to summarize, here's what I think:
> > >
> > >  - Each commit should be an atomic unit of code and be described by a
> > > descriptive commit subject and body.
> > >  - Each PR should be a logical grouping of closely related work;
> > > usually a single commit.
> > >  - We should use Rebase for flattening our changes.
> > >  - Squash is still an "okay" heuristic that accomplishes the same
> > > thing, most of the time.
> > > On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <de...@apache.org>
> > wrote:
> > > >
> > > > +1 based upon the fact that the Changelog.md can't seem to stay
> > current, we
> > > > do need to be able to maybe at least generate one from the commit
> > messages
> > > > within that release window.
> > > >
> > > > -Dew
> > > >
> > > > On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <
> > Jonathan_Gray@comcast.com>
> > > > wrote:
> > > >
> > > > > +1
> > > > >
> > > > >  In a past life when using Microsoft VSTS with git, the default
> > action was
> > > > > squash.  The rule of thumb we used was squash by default and only
> > merge if
> > > > > every commit was meaningful and required to understand what you were
> > trying
> > > > > to do.  We didn't typically have communal feature branches, so it
> > wasn't as
> > > > > large a use case for us.  The repo forensics are nice with merge,
> > but the
> > > > > reality we found was that git blame was more useful when we could
> > track to
> > > > > the PR rather than a mid-stream commit and then having to make a
> > second
> > > > > jump to the PR from there.  It also cut down on back/forth design
> > change
> > > > > commits and helped us stay on track with 1 PR == 1 issue because it
> > made
> > > > > unrelated fixes more apparent.
> > > > >
> > > > > Jonathan G
> > > > >
> > > > >
> > > > > On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com>
> > > > > wrote:
> > > > >
> > > > >     I'm not gonna say +1 or -1, but I'd still like to chime in. I
> > think
> > > > > it'd be nice to require PRs to squash things into atomic chunks. No
> > commits
> > > > > that say "started x" or "minor fixes", but idk if it's reasonable to
> > be
> > > > > able to boil every PR forever down to 70 characters. And sure you
> > can make
> > > > > multiple PRs if changes are too big to fit in one commit message,
> > but then
> > > > > it becomes really easy to become entwined in a chain of dependent
> > PRs so
> > > > > that every time one of them gets merged you have to rebase them all.
> > > > >
> > > > >     Of course, a "reasonable squash" is pretty hard to define
> > absolutely,
> > > > > and could actually just make PRs harder to review -  maybe it's not
> > even
> > > > > enforceable at all.
> > > > >     ________________________________________
> > > > >     From: Rawlin Peters <ra...@gmail.com>
> > > > >     Sent: Tuesday, October 23, 2018 11:14 AM
> > > > >     To: dev@trafficcontrol.apache.org
> > > > >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> > > > >
> > > > >     +1 on enabling squash and merge.
> > > > >
> > > > >     This will not only make cherry picks easier to manage with
> > respect to
> > > > >     merge conflicts, but it will also make the git log more useful
> > because
> > > > >     we will be able to see the full changeset of the PR in a single
> > commit
> > > > >     of the git log. Ideally, I'd also like the squashed commit to
> > contain
> > > > >     a link to the actual github PR so that I can easily view
> > comments, but
> > > > >     that is a proposal for another day.
> > > > >
> > > > >     - Rawlin
> > > > >
> > > > >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org>
> > > > > wrote:
> > > > >     >
> > > > >     > This seems to have stalled, so let me try to move it along.
> > > > >     >
> > > > >     > There is an ability to do "squash and merge" in github, we
> > just need
> > > > > to
> > > > >     > agree that we want to enable it for our project.
> > > > >     >
> > > > >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure
> > exactly how
> > > > > to
> > > > >     > handle the multiple committers thing, but I assume we can
> > squash
> > > > > into two
> > > > >     > commits and give one to each.  However, if we are doing
> > something big
> > > > >     > enough to require that many commits from multiple users, it
> > may be
> > > > > best as
> > > > >     > many PRs?  I don't think this is a very common situation, so I
> > would
> > > > > like
> > > > >     > to avoid getting bogged down in what-ifs if possible.
> > > > >     >
> > > > >     > As for the Changelog, I am fine with enabling squash and merge
> > and
> > > > > keeping
> > > > >     > the changelog as-is while we decide if squash and merge meets
> > our
> > > > > needs.  I
> > > > >     > am also game to revisit different ways to do it, and I
> > shouldn't have
> > > > >     > conflated that conversation with this one.  If someone wants
> > to see
> > > > > that
> > > > >     > changed, we should start a new discussion.
> > > > >     >
> > > > >     > Leif brings up a good point about squash and merge making
> > cherry
> > > > > picks
> > > > >     > easier so I think we should consider it for that point alone.
> > > > >     >
> > > > >     > So, what I am really asking is:  "do we think it is a good
> > idea to
> > > > > enable
> > > > >     > squash and merge on our Github repo so that we can reduce the
> > amount
> > > > > of
> > > > >     > commits per PR?"  I would like to get consensus for or
> > against.  If
> > > > > it is
> > > > >     > not clear already, I am for.
> > > > >     >
> > > > >     >
> > > > >     > Thanks,
> > > > >     > Dave
> > > > >     >
> > > > >     >
> > > > >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > > > > Derek_Gelinas@comcast.com>
> > > > >     > wrote:
> > > > >     >
> > > > >     > > I'll be honest, I think the simplest thing is still to just
> > > > > require a
> > > > >     > > changelog message, added to a central file, to be included
> > in the
> > > > > commit.
> > > > >     > > We've been going over and over this for months instead of
> > actually
> > > > > writing
> > > > >     > > change logs.  Let's just propose it and put it to a vote.
> > > > >     > >
> > > > >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > > > > rawlin.peters@gmail.com>
> > > > >     > > wrote:
> > > > >     > > >
> > > > >     > > > I'm +1 on squashing and merging, but I think we need a
> > > > > combination of
> > > > >     > > > a number of things to really have a decent changelog. Even
> > if we
> > > > > have
> > > > >     > > > awesome commit messages and squashed PRs, there's still
> > way too
> > > > > many
> > > > >     > > > PRs in a release to just generate a decent changelog from
> > the PR
> > > > >     > > > titles and/or commit messages. We still need a way to
> > logically
> > > > >     > > > separate PRs into bugfixes, new features, breaking
> > changes, etc.
> > > > > We
> > > > >     > > > could get some of that separation using github tags, but
> > what if
> > > > > there
> > > > >     > > > are multiple PRs that compose one new high-level feature?
> > At that
> > > > >     > > > point I think we'd have to create a new tag for every new
> > > > > high-level
> > > > >     > > > feature being worked on in order to group those PRs under
> > the
> > > > > same
> > > > >     > > > heading. There's also the problem that only committers can
> > tag
> > > > > issues,
> > > > >     > > > so it adds to the burden of committers to make sure all
> > > > > non-committer
> > > > >     > > > issues/PRs are properly tagged.
> > > > >     > > >
> > > > >     > > > The main headache with doing a manually-curated changelog
> > is the
> > > > > merge
> > > > >     > > > conflicts it causes, but I think we can easily avoid that
> > > > > problem by
> > > > >     > > > using a similar approach to reno [1] which breaks
> > individual
> > > > > notes out
> > > > >     > > > into their own separate yaml files which are then parsed
> > and
> > > > > merged
> > > > >     > > > into a single, cohesive changelog for each release. What
> > that
> > > > > means is
> > > > >     > > > basically every logical bugfix/new feature/upgrade note
> > gets its
> > > > > own
> > > > >     > > > file which avoids merge conflicts caused by just adding a
> > line
> > > > > to the
> > > > >     > > > end of a CHANGELOG.md file every time.
> > > > >     > > >
> > > > >     > > > - Rawlin
> > > > >     > > >
> > > > >     > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> > > > >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > > > > mitchell852@gmail.com>
> > > > >     > > wrote:
> > > > >     > > >>
> > > > >     > > >> is there a setting in github to enable "squash and merge"
> > vs
> > > > > "rebase and
> > > > >     > > >> merge"?
> > > > >     > > >>
> > > > >     > > >> i do have a couple of concerns however:
> > > > >     > > >>
> > > > >     > > >> 1. what happens when 2+ people contribute to a PR? This
> > is a
> > > > > pretty
> > > > >     > > common
> > > > >     > > >> scenario. If Bob and Sam both work on PR#1 is all the
> > commit
> > > > > history for
> > > > >     > > >> Sam lost?
> > > > >     > > >> 2. i can see benefits of having granular history...to a
> > point...
> > > > >     > > >>
> > > > >     > > >> The more I think about this. Why is squash needed?
> > Commits are
> > > > > grouped
> > > > >     > > by
> > > > >     > > >> PRs. Why isn't our change log just a list of PR merged
> > since
> > > > > the last
> > > > >     > > >> release? If a PR represents a working unit (as it
> > should), PR
> > > > > titles
> > > > >     > > should
> > > > >     > > >> be sufficient, no?
> > > > >     > > >>
> > > > >     > > >> For example, this is what our internal release change log
> > looks
> > > > > like.
> > > > >     > > >>
> > > > >     > > >> Changelog from 2526569a to 284555703
> > > > >     > > >> ========================================
> > > > >     > > >>
> > > > >     > > >> These changes are deployed in branch deploy-20181002.
> > > > >     > > >>
> > > > >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > > > >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> > > > >     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18
> > 2018)
> > > > >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> > > > >     > > cachegroups/dses
> > > > >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > > > >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO
> > > > > unused func
> > > > >     > > >> (Robert Butts, Sep 13 2018)
> > > > >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > > > >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> > > > >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > > > >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO
> > API
> > > > > Tests for
> > > > >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> > > > >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go
> > > > > client
> > > > >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > > > >     > > >> PR 2806: Add sub to validate user input when running
> > > > > postinstall script
> > > > >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub
> > to
> > > > > validate
> > > > >     > > user
> > > > >     > > >> input when running postinstall script changes default cdn
> > name
> > > > > to not
> > > > >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > > > >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering
> > > > > postinstall
> > > > >     > > cdn
> > > > >     > > >> name regex to include underscores (Dylan Souza, Sep 21
> > 2018)
> > > > >     > > >>        70d8893335b63db006974932341378717c42701c: Changing
> > back
> > > > > the
> > > > >     > > default
> > > > >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> > > > >     > > >> PR 2812: remove traffic_monitor_java
> > > > >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> > > > >     > > traffic_monitor_java
> > > > >     > > >> (Dan Kirkwood, Sep 12 2018)
> > > > >     > > >>
> > > > >     > > >>
> > > > >     > > >> [truncated]
> > > > >     > > >>
> > > > >     > > >>
> > > > >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <
> > neuman@apache.org>
> > > > > wrote:
> > > > >     > > >>>
> > > > >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > > > > information on what
> > > > >     > > >>> actually went into the release, which therefore makes it
> > not
> > > > > useful.
> > > > >     > > If we
> > > > >     > > >>> were better about commit messages and used squash and
> > merge,
> > > > > then we
> > > > >     > > would
> > > > >     > > >>> at least have something for every PR that was merged.
> > > > >     > > >>>
> > > > >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <
> > jvd@knutsel.com>
> > > > > wrote:
> > > > >     > > >>>
> > > > >     > > >>>>> I think this just shifts the burden from writing a
> > changelog
> > > > > entry to
> > > > >     > > >>>> writing a good commit entry.
> > > > >     > > >>>>
> > > > >     > > >>>>
> > > > >     > > >>>> I agree, and I think that’s where that burden belongs.
> > (And I
> > > > > _really_
> > > > >     > > >>>> hate merge conflicts).
> > > > >     > > >>>>
> > > > >     > > >>>>> There might be fewer commit entries if we squash and
> > merge,
> > > > > but I’m
> > > > >     > > >>>> doubtful that it would be as valuable as our “curated”
> > > > > changelogs.
> > > > >     > > >>>>
> > > > >     > > >>>> Both are dependent on how disciplined we are.
> > > > >     > > >>>>
> > > > >     > > >>>> I’m +1 on Squash and Merge.
> > > > >     > > >>>>
> > > > >     > > >>>> Cheers,
> > > > >     > > >>>> JvD
> > > > >     > > >>>>
> > > > >     > > >>>>
> > > > >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > > > >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> > > > >     > > >>>>>
> > > > >     > > >>>>> I think this just shifts the burden from writing a
> > changelog
> > > > > entry to
> > > > >     > > >>>> writing a good commit entry.
> > > > >     > > >>>>>
> > > > >     > > >>>>> There might be fewer commit entries if we squash and
> > merge,
> > > > > but I’m
> > > > >     > > >>>> doubtful that it would be as valuable as our “curated”
> > > > > changelogs.
> > > > >     > > >>>>>
> > > > >     > > >>>>>
> > > > >     > > >>>>>
> > > > >     > > >>>>>
> > > > >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <
> > neuman@apache.org>
> > > > > wrote:
> > > > >     > > >>>>>>
> > > > >     > > >>>>>> Hey All,
> > > > >     > > >>>>>> I want to follow up on something that was briefly
> > discussed
> > > > > at the
> > > > >     > > >>>> summit
> > > > >     > > >>>>>> this week.  Many people do not like the Changelog and
> > feel
> > > > > like it
> > > > >     > > can
> > > > >     > > >>>> be a
> > > > >     > > >>>>>> PITA to deal with.  One of the reasons we have the
> > > > > changelog is
> > > > >     > > >>> because
> > > > >     > > >>>>>> there are so many commits in a given release, that it
> > is
> > > > > hard to
> > > > >     > > comb
> > > > >     > > >>>>>> through all of them to figure out what noteworthy
> > changes
> > > > > or bug
> > > > >     > > fixes
> > > > >     > > >>>> went
> > > > >     > > >>>>>> into the code.  One thing that may help with this
> > problem
> > > > > is to use
> > > > >     > > >>>> enable
> > > > >     > > >>>>>> the squash and merge feature on Github for pull
> > requests
> > > > > [1].   This
> > > > >     > > >>>>>> feature would squash all commits in a PR into one
> > commit.
> > > > > If we
> > > > >     > > pair
> > > > >     > > >>>> the
> > > > >     > > >>>>>> one commit with a good commit message, we would
> > essentially
> > > > > get the
> > > > >     > > >>>> ability
> > > > >     > > >>>>>> to create the changelog just from the commits.
> > > > >     > > >>>>>>
> > > > >     > > >>>>>> So, I would like to propose that we enable the squash
> > and
> > > > > merge
> > > > >     > > >>> feature
> > > > >     > > >>>> in
> > > > >     > > >>>>>> the Traffic Control Github repo and use that going
> > forward.
> > > > >     > > Thoughts?
> > > > >     > > >>>>>>
> > > > >     > > >>>>>> Thanks,
> > > > >     > > >>>>>> Dave
> > > > >     > > >>>>>>
> > > > >     > > >>>>>>
> > > > >     > > >>>>>> [1]
> > > > > https://help.github.com/articles/about-pull-request-merges/
> > > > >     > > >>>>>
> > > > >     > > >>>>
> > > > >     > > >>>>
> > > > >     > > >>>
> > > > >     > >
> > > > >
> > > > >
> > > > >
> >

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Jeremy Mitchell <mi...@gmail.com>]

I do like the idea of adding the 'squash and merge` option so we end up
with 2 Github options:

- rebase and merge
- squash and merge

Leave it up to the merger which option to choose.

Now as far as how to create a "change log", back to what Leif said earlier:

"Why not generate the ChangeLog from the actual PR “subject” line? Then it
wouldn’t matter if you have 1 or 100 commits for a particular PR. The
Sorber wrote a little script for ATS that generates our ChangeLogs *)"



On Wed, Oct 24, 2018 at 10:24 AM Rawlin Peters <ra...@gmail.com>
wrote:

> I agree with most of this and wish everyone would structure their PRs
> and commits like this, but the fact of the matter is that small
> checkpoint commits are simply just the way some people like to work.
> For the last few things I've worked on, I've tried to separate changes
> to the different components for the same logical feature into
> different PRs (API, UI, Traffic Router). I find that breaking down the
> feature into a PR per component makes things a bit easier to review,
> since not everyone has experience all the various TC components that
> could be mashed into one mega-PR. If a mega-PR contains changes to TO,
> TP, and TR all in one, some people maybe be comfortable reviewing TO
> but not necessarily TP or TR, so you end up having to coordinate with
> multiple reviewers/committers who are comfortable reviewing different
> areas just to get a single PR merged.
>
> Unless we start making good commit messages and "one standalone
> change" per commit a hard requirement for all contributors to the
> project, I doubt we can ever get to this ideal, and I'm not sure we
> should make that a requirement right now. However, by squashing and
> merging commits, we can clean up the git history by removing all of
> the non-atomic/checkpoint commits as well as other benefits that have
> already been discussed.
>
> Nonetheless, we will still be able to use the "rebase and merge"
> option at the committer's discretion. If the contributor and committer
> feel that the commits in the PR are more valuable in the history
> because they're atomic and each have good commit messages, the
> committer should choose the "rebase and merge" option. If the PR
> contains lots of small, nonatomic, checkpoint commits that would
> obfuscate the git history and make it harder for someone doing a `git
> blame` to figure out why a change was made, then maybe it would be
> better for the committer to choose the "squash and merge" option or
> kindly ask the PR submitter to squash the commits themselves into a
> single commit and write a good title+message.
>
> - Rawlin
>
> On Tue, Oct 23, 2018 at 4:49 PM Chris Lemmons <al...@gmail.com> wrote:
> >
> > I don't think I'm quite -1 on this, but I think we can do better.
> >
> > I think a PR should be a single unit of work and a commit should be a
> > single unit of code.
> >
> > I like using atomic commits. That is, each commit does one thing. So
> > if I need to revert a fix or track down when something was introduced,
> > I can find it. For commit messages, I really like the seven rules
> > documented here, but especially the last one:
> > https://chris.beams.io/posts/git-commit/
> >
> > > Use the body to explain what and why vs. how
> >
> > I think it's important to put the body in the commit message in order
> > to explain why the change is necessary what the non-obvious effects of
> > it are. Having a good subject for the commit is also very useful,
> > since that's what GitHub (and really all the tools) shows when you
> > scroll through commits, looking for what changed.
> >
> > In order to have atomic commits, sometimes you'll need one or more
> > commits as part of a single PR. Not most of the time, but sometimes.
> > Especially with a large feature, you may make multiple dependent
> > changes, but each one is a logical unit of work. For example, if the
> > task is "add support for foobaz plugin", there might be three commits,
> > with summaries that look like "Add support for foobaz plugin to config
> > generation", "Add UI for editing foobaz plugin config", and "Add API
> > endpoints for foobaz plugin config". That also makes it easy to direct
> > domain experts to each of the changesets for focused review.
> >
> > A PR on the other hand, is a single unit of work. While each commit
> > should be stand-alone, capable of running correctly before and after,
> > each commit might not be desirable on a standalone basis. For example,
> > while "Add support for foobaz plugin to config generation" is a single
> > unit of code in that it operates correctly, has tests, is documented,
> > and is completely implemented, it's not exceptionally useful without a
> > UI to edit it. It makes sense to group all these changes and merge
> > them as a unit.
> >
> > For almost all bug fixes and most medium to small features, each PR
> > will have precisely one commit. But especially for larger "projects",
> > it makes the most sense to group it all as one PR, but merge it as a
> > feature.
> >
> > Relatedly, it's very rarely correct to merge commits from two
> > different authors. (This is important for legal reasons as well, if it
> > ever came to it; authors license their code when they contribute it,
> > so it's important to know whence it came.) However, in the event that
> > two people jointly produce what amounts to a single unit of code (for
> > example, by pair programming, or by collaborating on a snippet of code
> > on Slack or IRC), you can document that in git by adding it to the end
> > of the commit message body:
> >
> > > Co-authored-by: name <na...@example.com>
> > > Co-authored-by: another-name <an...@example.com>"
> >
> > Details for that technique here:
> >
> https://help.github.com/articles/creating-a-commit-with-multiple-authors/
> >
> > It's not quite a "standard", but it's quickly being adopted by most of
> > the tools I use, at least.
> >
> > As for the "Squash" option, I actually prefer the "Rebase" choice.
> > It's the same thing if you have single-commit PRs, but it handles
> > multi-commit PRs a bit more nicely.
> >
> > So, to summarize, here's what I think:
> >
> >  - Each commit should be an atomic unit of code and be described by a
> > descriptive commit subject and body.
> >  - Each PR should be a logical grouping of closely related work;
> > usually a single commit.
> >  - We should use Rebase for flattening our changes.
> >  - Squash is still an "okay" heuristic that accomplishes the same
> > thing, most of the time.
> > On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <de...@apache.org>
> wrote:
> > >
> > > +1 based upon the fact that the Changelog.md can't seem to stay
> current, we
> > > do need to be able to maybe at least generate one from the commit
> messages
> > > within that release window.
> > >
> > > -Dew
> > >
> > > On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <
> Jonathan_Gray@comcast.com>
> > > wrote:
> > >
> > > > +1
> > > >
> > > >  In a past life when using Microsoft VSTS with git, the default
> action was
> > > > squash.  The rule of thumb we used was squash by default and only
> merge if
> > > > every commit was meaningful and required to understand what you were
> trying
> > > > to do.  We didn't typically have communal feature branches, so it
> wasn't as
> > > > large a use case for us.  The repo forensics are nice with merge,
> but the
> > > > reality we found was that git blame was more useful when we could
> track to
> > > > the PR rather than a mid-stream commit and then having to make a
> second
> > > > jump to the PR from there.  It also cut down on back/forth design
> change
> > > > commits and helped us stay on track with 1 PR == 1 issue because it
> made
> > > > unrelated fixes more apparent.
> > > >
> > > > Jonathan G
> > > >
> > > >
> > > > On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com>
> > > > wrote:
> > > >
> > > >     I'm not gonna say +1 or -1, but I'd still like to chime in. I
> think
> > > > it'd be nice to require PRs to squash things into atomic chunks. No
> commits
> > > > that say "started x" or "minor fixes", but idk if it's reasonable to
> be
> > > > able to boil every PR forever down to 70 characters. And sure you
> can make
> > > > multiple PRs if changes are too big to fit in one commit message,
> but then
> > > > it becomes really easy to become entwined in a chain of dependent
> PRs so
> > > > that every time one of them gets merged you have to rebase them all.
> > > >
> > > >     Of course, a "reasonable squash" is pretty hard to define
> absolutely,
> > > > and could actually just make PRs harder to review -  maybe it's not
> even
> > > > enforceable at all.
> > > >     ________________________________________
> > > >     From: Rawlin Peters <ra...@gmail.com>
> > > >     Sent: Tuesday, October 23, 2018 11:14 AM
> > > >     To: dev@trafficcontrol.apache.org
> > > >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> > > >
> > > >     +1 on enabling squash and merge.
> > > >
> > > >     This will not only make cherry picks easier to manage with
> respect to
> > > >     merge conflicts, but it will also make the git log more useful
> because
> > > >     we will be able to see the full changeset of the PR in a single
> commit
> > > >     of the git log. Ideally, I'd also like the squashed commit to
> contain
> > > >     a link to the actual github PR so that I can easily view
> comments, but
> > > >     that is a proposal for another day.
> > > >
> > > >     - Rawlin
> > > >
> > > >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org>
> > > > wrote:
> > > >     >
> > > >     > This seems to have stalled, so let me try to move it along.
> > > >     >
> > > >     > There is an ability to do "squash and merge" in github, we
> just need
> > > > to
> > > >     > agree that we want to enable it for our project.
> > > >     >
> > > >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure
> exactly how
> > > > to
> > > >     > handle the multiple committers thing, but I assume we can
> squash
> > > > into two
> > > >     > commits and give one to each.  However, if we are doing
> something big
> > > >     > enough to require that many commits from multiple users, it
> may be
> > > > best as
> > > >     > many PRs?  I don't think this is a very common situation, so I
> would
> > > > like
> > > >     > to avoid getting bogged down in what-ifs if possible.
> > > >     >
> > > >     > As for the Changelog, I am fine with enabling squash and merge
> and
> > > > keeping
> > > >     > the changelog as-is while we decide if squash and merge meets
> our
> > > > needs.  I
> > > >     > am also game to revisit different ways to do it, and I
> shouldn't have
> > > >     > conflated that conversation with this one.  If someone wants
> to see
> > > > that
> > > >     > changed, we should start a new discussion.
> > > >     >
> > > >     > Leif brings up a good point about squash and merge making
> cherry
> > > > picks
> > > >     > easier so I think we should consider it for that point alone.
> > > >     >
> > > >     > So, what I am really asking is:  "do we think it is a good
> idea to
> > > > enable
> > > >     > squash and merge on our Github repo so that we can reduce the
> amount
> > > > of
> > > >     > commits per PR?"  I would like to get consensus for or
> against.  If
> > > > it is
> > > >     > not clear already, I am for.
> > > >     >
> > > >     >
> > > >     > Thanks,
> > > >     > Dave
> > > >     >
> > > >     >
> > > >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > > > Derek_Gelinas@comcast.com>
> > > >     > wrote:
> > > >     >
> > > >     > > I'll be honest, I think the simplest thing is still to just
> > > > require a
> > > >     > > changelog message, added to a central file, to be included
> in the
> > > > commit.
> > > >     > > We've been going over and over this for months instead of
> actually
> > > > writing
> > > >     > > change logs.  Let's just propose it and put it to a vote.
> > > >     > >
> > > >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > > > rawlin.peters@gmail.com>
> > > >     > > wrote:
> > > >     > > >
> > > >     > > > I'm +1 on squashing and merging, but I think we need a
> > > > combination of
> > > >     > > > a number of things to really have a decent changelog. Even
> if we
> > > > have
> > > >     > > > awesome commit messages and squashed PRs, there's still
> way too
> > > > many
> > > >     > > > PRs in a release to just generate a decent changelog from
> the PR
> > > >     > > > titles and/or commit messages. We still need a way to
> logically
> > > >     > > > separate PRs into bugfixes, new features, breaking
> changes, etc.
> > > > We
> > > >     > > > could get some of that separation using github tags, but
> what if
> > > > there
> > > >     > > > are multiple PRs that compose one new high-level feature?
> At that
> > > >     > > > point I think we'd have to create a new tag for every new
> > > > high-level
> > > >     > > > feature being worked on in order to group those PRs under
> the
> > > > same
> > > >     > > > heading. There's also the problem that only committers can
> tag
> > > > issues,
> > > >     > > > so it adds to the burden of committers to make sure all
> > > > non-committer
> > > >     > > > issues/PRs are properly tagged.
> > > >     > > >
> > > >     > > > The main headache with doing a manually-curated changelog
> is the
> > > > merge
> > > >     > > > conflicts it causes, but I think we can easily avoid that
> > > > problem by
> > > >     > > > using a similar approach to reno [1] which breaks
> individual
> > > > notes out
> > > >     > > > into their own separate yaml files which are then parsed
> and
> > > > merged
> > > >     > > > into a single, cohesive changelog for each release. What
> that
> > > > means is
> > > >     > > > basically every logical bugfix/new feature/upgrade note
> gets its
> > > > own
> > > >     > > > file which avoids merge conflicts caused by just adding a
> line
> > > > to the
> > > >     > > > end of a CHANGELOG.md file every time.
> > > >     > > >
> > > >     > > > - Rawlin
> > > >     > > >
> > > >     > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> > > >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > > > mitchell852@gmail.com>
> > > >     > > wrote:
> > > >     > > >>
> > > >     > > >> is there a setting in github to enable "squash and merge"
> vs
> > > > "rebase and
> > > >     > > >> merge"?
> > > >     > > >>
> > > >     > > >> i do have a couple of concerns however:
> > > >     > > >>
> > > >     > > >> 1. what happens when 2+ people contribute to a PR? This
> is a
> > > > pretty
> > > >     > > common
> > > >     > > >> scenario. If Bob and Sam both work on PR#1 is all the
> commit
> > > > history for
> > > >     > > >> Sam lost?
> > > >     > > >> 2. i can see benefits of having granular history...to a
> point...
> > > >     > > >>
> > > >     > > >> The more I think about this. Why is squash needed?
> Commits are
> > > > grouped
> > > >     > > by
> > > >     > > >> PRs. Why isn't our change log just a list of PR merged
> since
> > > > the last
> > > >     > > >> release? If a PR represents a working unit (as it
> should), PR
> > > > titles
> > > >     > > should
> > > >     > > >> be sufficient, no?
> > > >     > > >>
> > > >     > > >> For example, this is what our internal release change log
> looks
> > > > like.
> > > >     > > >>
> > > >     > > >> Changelog from 2526569a to 284555703
> > > >     > > >> ========================================
> > > >     > > >>
> > > >     > > >> These changes are deployed in branch deploy-20181002.
> > > >     > > >>
> > > >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > > >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> > > >     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18
> 2018)
> > > >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> > > >     > > cachegroups/dses
> > > >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > > >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO
> > > > unused func
> > > >     > > >> (Robert Butts, Sep 13 2018)
> > > >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > > >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> > > >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > > >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO
> API
> > > > Tests for
> > > >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> > > >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go
> > > > client
> > > >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > > >     > > >> PR 2806: Add sub to validate user input when running
> > > > postinstall script
> > > >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub
> to
> > > > validate
> > > >     > > user
> > > >     > > >> input when running postinstall script changes default cdn
> name
> > > > to not
> > > >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > > >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering
> > > > postinstall
> > > >     > > cdn
> > > >     > > >> name regex to include underscores (Dylan Souza, Sep 21
> 2018)
> > > >     > > >>        70d8893335b63db006974932341378717c42701c: Changing
> back
> > > > the
> > > >     > > default
> > > >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> > > >     > > >> PR 2812: remove traffic_monitor_java
> > > >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> > > >     > > traffic_monitor_java
> > > >     > > >> (Dan Kirkwood, Sep 12 2018)
> > > >     > > >>
> > > >     > > >>
> > > >     > > >> [truncated]
> > > >     > > >>
> > > >     > > >>
> > > >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <
> neuman@apache.org>
> > > > wrote:
> > > >     > > >>>
> > > >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > > > information on what
> > > >     > > >>> actually went into the release, which therefore makes it
> not
> > > > useful.
> > > >     > > If we
> > > >     > > >>> were better about commit messages and used squash and
> merge,
> > > > then we
> > > >     > > would
> > > >     > > >>> at least have something for every PR that was merged.
> > > >     > > >>>
> > > >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <
> jvd@knutsel.com>
> > > > wrote:
> > > >     > > >>>
> > > >     > > >>>>> I think this just shifts the burden from writing a
> changelog
> > > > entry to
> > > >     > > >>>> writing a good commit entry.
> > > >     > > >>>>
> > > >     > > >>>>
> > > >     > > >>>> I agree, and I think that’s where that burden belongs.
> (And I
> > > > _really_
> > > >     > > >>>> hate merge conflicts).
> > > >     > > >>>>
> > > >     > > >>>>> There might be fewer commit entries if we squash and
> merge,
> > > > but I’m
> > > >     > > >>>> doubtful that it would be as valuable as our “curated”
> > > > changelogs.
> > > >     > > >>>>
> > > >     > > >>>> Both are dependent on how disciplined we are.
> > > >     > > >>>>
> > > >     > > >>>> I’m +1 on Squash and Merge.
> > > >     > > >>>>
> > > >     > > >>>> Cheers,
> > > >     > > >>>> JvD
> > > >     > > >>>>
> > > >     > > >>>>
> > > >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > > >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> > > >     > > >>>>>
> > > >     > > >>>>> I think this just shifts the burden from writing a
> changelog
> > > > entry to
> > > >     > > >>>> writing a good commit entry.
> > > >     > > >>>>>
> > > >     > > >>>>> There might be fewer commit entries if we squash and
> merge,
> > > > but I’m
> > > >     > > >>>> doubtful that it would be as valuable as our “curated”
> > > > changelogs.
> > > >     > > >>>>>
> > > >     > > >>>>>
> > > >     > > >>>>>
> > > >     > > >>>>>
> > > >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <
> neuman@apache.org>
> > > > wrote:
> > > >     > > >>>>>>
> > > >     > > >>>>>> Hey All,
> > > >     > > >>>>>> I want to follow up on something that was briefly
> discussed
> > > > at the
> > > >     > > >>>> summit
> > > >     > > >>>>>> this week.  Many people do not like the Changelog and
> feel
> > > > like it
> > > >     > > can
> > > >     > > >>>> be a
> > > >     > > >>>>>> PITA to deal with.  One of the reasons we have the
> > > > changelog is
> > > >     > > >>> because
> > > >     > > >>>>>> there are so many commits in a given release, that it
> is
> > > > hard to
> > > >     > > comb
> > > >     > > >>>>>> through all of them to figure out what noteworthy
> changes
> > > > or bug
> > > >     > > fixes
> > > >     > > >>>> went
> > > >     > > >>>>>> into the code.  One thing that may help with this
> problem
> > > > is to use
> > > >     > > >>>> enable
> > > >     > > >>>>>> the squash and merge feature on Github for pull
> requests
> > > > [1].   This
> > > >     > > >>>>>> feature would squash all commits in a PR into one
> commit.
> > > > If we
> > > >     > > pair
> > > >     > > >>>> the
> > > >     > > >>>>>> one commit with a good commit message, we would
> essentially
> > > > get the
> > > >     > > >>>> ability
> > > >     > > >>>>>> to create the changelog just from the commits.
> > > >     > > >>>>>>
> > > >     > > >>>>>> So, I would like to propose that we enable the squash
> and
> > > > merge
> > > >     > > >>> feature
> > > >     > > >>>> in
> > > >     > > >>>>>> the Traffic Control Github repo and use that going
> forward.
> > > >     > > Thoughts?
> > > >     > > >>>>>>
> > > >     > > >>>>>> Thanks,
> > > >     > > >>>>>> Dave
> > > >     > > >>>>>>
> > > >     > > >>>>>>
> > > >     > > >>>>>> [1]
> > > > https://help.github.com/articles/about-pull-request-merges/
> > > >     > > >>>>>
> > > >     > > >>>>
> > > >     > > >>>>
> > > >     > > >>>
> > > >     > >
> > > >
> > > >
> > > >
>

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Rawlin Peters <ra...@gmail.com>]

I agree with most of this and wish everyone would structure their PRs
and commits like this, but the fact of the matter is that small
checkpoint commits are simply just the way some people like to work.
For the last few things I've worked on, I've tried to separate changes
to the different components for the same logical feature into
different PRs (API, UI, Traffic Router). I find that breaking down the
feature into a PR per component makes things a bit easier to review,
since not everyone has experience all the various TC components that
could be mashed into one mega-PR. If a mega-PR contains changes to TO,
TP, and TR all in one, some people maybe be comfortable reviewing TO
but not necessarily TP or TR, so you end up having to coordinate with
multiple reviewers/committers who are comfortable reviewing different
areas just to get a single PR merged.

Unless we start making good commit messages and "one standalone
change" per commit a hard requirement for all contributors to the
project, I doubt we can ever get to this ideal, and I'm not sure we
should make that a requirement right now. However, by squashing and
merging commits, we can clean up the git history by removing all of
the non-atomic/checkpoint commits as well as other benefits that have
already been discussed.

Nonetheless, we will still be able to use the "rebase and merge"
option at the committer's discretion. If the contributor and committer
feel that the commits in the PR are more valuable in the history
because they're atomic and each have good commit messages, the
committer should choose the "rebase and merge" option. If the PR
contains lots of small, nonatomic, checkpoint commits that would
obfuscate the git history and make it harder for someone doing a `git
blame` to figure out why a change was made, then maybe it would be
better for the committer to choose the "squash and merge" option or
kindly ask the PR submitter to squash the commits themselves into a
single commit and write a good title+message.

- Rawlin

On Tue, Oct 23, 2018 at 4:49 PM Chris Lemmons <al...@gmail.com> wrote:
>
> I don't think I'm quite -1 on this, but I think we can do better.
>
> I think a PR should be a single unit of work and a commit should be a
> single unit of code.
>
> I like using atomic commits. That is, each commit does one thing. So
> if I need to revert a fix or track down when something was introduced,
> I can find it. For commit messages, I really like the seven rules
> documented here, but especially the last one:
> https://chris.beams.io/posts/git-commit/
>
> > Use the body to explain what and why vs. how
>
> I think it's important to put the body in the commit message in order
> to explain why the change is necessary what the non-obvious effects of
> it are. Having a good subject for the commit is also very useful,
> since that's what GitHub (and really all the tools) shows when you
> scroll through commits, looking for what changed.
>
> In order to have atomic commits, sometimes you'll need one or more
> commits as part of a single PR. Not most of the time, but sometimes.
> Especially with a large feature, you may make multiple dependent
> changes, but each one is a logical unit of work. For example, if the
> task is "add support for foobaz plugin", there might be three commits,
> with summaries that look like "Add support for foobaz plugin to config
> generation", "Add UI for editing foobaz plugin config", and "Add API
> endpoints for foobaz plugin config". That also makes it easy to direct
> domain experts to each of the changesets for focused review.
>
> A PR on the other hand, is a single unit of work. While each commit
> should be stand-alone, capable of running correctly before and after,
> each commit might not be desirable on a standalone basis. For example,
> while "Add support for foobaz plugin to config generation" is a single
> unit of code in that it operates correctly, has tests, is documented,
> and is completely implemented, it's not exceptionally useful without a
> UI to edit it. It makes sense to group all these changes and merge
> them as a unit.
>
> For almost all bug fixes and most medium to small features, each PR
> will have precisely one commit. But especially for larger "projects",
> it makes the most sense to group it all as one PR, but merge it as a
> feature.
>
> Relatedly, it's very rarely correct to merge commits from two
> different authors. (This is important for legal reasons as well, if it
> ever came to it; authors license their code when they contribute it,
> so it's important to know whence it came.) However, in the event that
> two people jointly produce what amounts to a single unit of code (for
> example, by pair programming, or by collaborating on a snippet of code
> on Slack or IRC), you can document that in git by adding it to the end
> of the commit message body:
>
> > Co-authored-by: name <na...@example.com>
> > Co-authored-by: another-name <an...@example.com>"
>
> Details for that technique here:
> https://help.github.com/articles/creating-a-commit-with-multiple-authors/
>
> It's not quite a "standard", but it's quickly being adopted by most of
> the tools I use, at least.
>
> As for the "Squash" option, I actually prefer the "Rebase" choice.
> It's the same thing if you have single-commit PRs, but it handles
> multi-commit PRs a bit more nicely.
>
> So, to summarize, here's what I think:
>
>  - Each commit should be an atomic unit of code and be described by a
> descriptive commit subject and body.
>  - Each PR should be a logical grouping of closely related work;
> usually a single commit.
>  - We should use Rebase for flattening our changes.
>  - Squash is still an "okay" heuristic that accomplishes the same
> thing, most of the time.
> On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <de...@apache.org> wrote:
> >
> > +1 based upon the fact that the Changelog.md can't seem to stay current, we
> > do need to be able to maybe at least generate one from the commit messages
> > within that release window.
> >
> > -Dew
> >
> > On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <Jo...@comcast.com>
> > wrote:
> >
> > > +1
> > >
> > >  In a past life when using Microsoft VSTS with git, the default action was
> > > squash.  The rule of thumb we used was squash by default and only merge if
> > > every commit was meaningful and required to understand what you were trying
> > > to do.  We didn't typically have communal feature branches, so it wasn't as
> > > large a use case for us.  The repo forensics are nice with merge, but the
> > > reality we found was that git blame was more useful when we could track to
> > > the PR rather than a mid-stream commit and then having to make a second
> > > jump to the PR from there.  It also cut down on back/forth design change
> > > commits and helped us stay on track with 1 PR == 1 issue because it made
> > > unrelated fixes more apparent.
> > >
> > > Jonathan G
> > >
> > >
> > > On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com>
> > > wrote:
> > >
> > >     I'm not gonna say +1 or -1, but I'd still like to chime in. I think
> > > it'd be nice to require PRs to squash things into atomic chunks. No commits
> > > that say "started x" or "minor fixes", but idk if it's reasonable to be
> > > able to boil every PR forever down to 70 characters. And sure you can make
> > > multiple PRs if changes are too big to fit in one commit message, but then
> > > it becomes really easy to become entwined in a chain of dependent PRs so
> > > that every time one of them gets merged you have to rebase them all.
> > >
> > >     Of course, a "reasonable squash" is pretty hard to define absolutely,
> > > and could actually just make PRs harder to review -  maybe it's not even
> > > enforceable at all.
> > >     ________________________________________
> > >     From: Rawlin Peters <ra...@gmail.com>
> > >     Sent: Tuesday, October 23, 2018 11:14 AM
> > >     To: dev@trafficcontrol.apache.org
> > >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> > >
> > >     +1 on enabling squash and merge.
> > >
> > >     This will not only make cherry picks easier to manage with respect to
> > >     merge conflicts, but it will also make the git log more useful because
> > >     we will be able to see the full changeset of the PR in a single commit
> > >     of the git log. Ideally, I'd also like the squashed commit to contain
> > >     a link to the actual github PR so that I can easily view comments, but
> > >     that is a proposal for another day.
> > >
> > >     - Rawlin
> > >
> > >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org>
> > > wrote:
> > >     >
> > >     > This seems to have stalled, so let me try to move it along.
> > >     >
> > >     > There is an ability to do "squash and merge" in github, we just need
> > > to
> > >     > agree that we want to enable it for our project.
> > >     >
> > >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how
> > > to
> > >     > handle the multiple committers thing, but I assume we can squash
> > > into two
> > >     > commits and give one to each.  However, if we are doing something big
> > >     > enough to require that many commits from multiple users, it may be
> > > best as
> > >     > many PRs?  I don't think this is a very common situation, so I would
> > > like
> > >     > to avoid getting bogged down in what-ifs if possible.
> > >     >
> > >     > As for the Changelog, I am fine with enabling squash and merge and
> > > keeping
> > >     > the changelog as-is while we decide if squash and merge meets our
> > > needs.  I
> > >     > am also game to revisit different ways to do it, and I shouldn't have
> > >     > conflated that conversation with this one.  If someone wants to see
> > > that
> > >     > changed, we should start a new discussion.
> > >     >
> > >     > Leif brings up a good point about squash and merge making cherry
> > > picks
> > >     > easier so I think we should consider it for that point alone.
> > >     >
> > >     > So, what I am really asking is:  "do we think it is a good idea to
> > > enable
> > >     > squash and merge on our Github repo so that we can reduce the amount
> > > of
> > >     > commits per PR?"  I would like to get consensus for or against.  If
> > > it is
> > >     > not clear already, I am for.
> > >     >
> > >     >
> > >     > Thanks,
> > >     > Dave
> > >     >
> > >     >
> > >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > > Derek_Gelinas@comcast.com>
> > >     > wrote:
> > >     >
> > >     > > I'll be honest, I think the simplest thing is still to just
> > > require a
> > >     > > changelog message, added to a central file, to be included in the
> > > commit.
> > >     > > We've been going over and over this for months instead of actually
> > > writing
> > >     > > change logs.  Let's just propose it and put it to a vote.
> > >     > >
> > >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > > rawlin.peters@gmail.com>
> > >     > > wrote:
> > >     > > >
> > >     > > > I'm +1 on squashing and merging, but I think we need a
> > > combination of
> > >     > > > a number of things to really have a decent changelog. Even if we
> > > have
> > >     > > > awesome commit messages and squashed PRs, there's still way too
> > > many
> > >     > > > PRs in a release to just generate a decent changelog from the PR
> > >     > > > titles and/or commit messages. We still need a way to logically
> > >     > > > separate PRs into bugfixes, new features, breaking changes, etc.
> > > We
> > >     > > > could get some of that separation using github tags, but what if
> > > there
> > >     > > > are multiple PRs that compose one new high-level feature? At that
> > >     > > > point I think we'd have to create a new tag for every new
> > > high-level
> > >     > > > feature being worked on in order to group those PRs under the
> > > same
> > >     > > > heading. There's also the problem that only committers can tag
> > > issues,
> > >     > > > so it adds to the burden of committers to make sure all
> > > non-committer
> > >     > > > issues/PRs are properly tagged.
> > >     > > >
> > >     > > > The main headache with doing a manually-curated changelog is the
> > > merge
> > >     > > > conflicts it causes, but I think we can easily avoid that
> > > problem by
> > >     > > > using a similar approach to reno [1] which breaks individual
> > > notes out
> > >     > > > into their own separate yaml files which are then parsed and
> > > merged
> > >     > > > into a single, cohesive changelog for each release. What that
> > > means is
> > >     > > > basically every logical bugfix/new feature/upgrade note gets its
> > > own
> > >     > > > file which avoids merge conflicts caused by just adding a line
> > > to the
> > >     > > > end of a CHANGELOG.md file every time.
> > >     > > >
> > >     > > > - Rawlin
> > >     > > >
> > >     > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> > >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > > mitchell852@gmail.com>
> > >     > > wrote:
> > >     > > >>
> > >     > > >> is there a setting in github to enable "squash and merge" vs
> > > "rebase and
> > >     > > >> merge"?
> > >     > > >>
> > >     > > >> i do have a couple of concerns however:
> > >     > > >>
> > >     > > >> 1. what happens when 2+ people contribute to a PR? This is a
> > > pretty
> > >     > > common
> > >     > > >> scenario. If Bob and Sam both work on PR#1 is all the commit
> > > history for
> > >     > > >> Sam lost?
> > >     > > >> 2. i can see benefits of having granular history...to a point...
> > >     > > >>
> > >     > > >> The more I think about this. Why is squash needed? Commits are
> > > grouped
> > >     > > by
> > >     > > >> PRs. Why isn't our change log just a list of PR merged since
> > > the last
> > >     > > >> release? If a PR represents a working unit (as it should), PR
> > > titles
> > >     > > should
> > >     > > >> be sufficient, no?
> > >     > > >>
> > >     > > >> For example, this is what our internal release change log looks
> > > like.
> > >     > > >>
> > >     > > >> Changelog from 2526569a to 284555703
> > >     > > >> ========================================
> > >     > > >>
> > >     > > >> These changes are deployed in branch deploy-20181002.
> > >     > > >>
> > >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> > >     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
> > >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> > >     > > cachegroups/dses
> > >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO
> > > unused func
> > >     > > >> (Robert Butts, Sep 13 2018)
> > >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> > >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API
> > > Tests for
> > >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> > >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go
> > > client
> > >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > >     > > >> PR 2806: Add sub to validate user input when running
> > > postinstall script
> > >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to
> > > validate
> > >     > > user
> > >     > > >> input when running postinstall script changes default cdn name
> > > to not
> > >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering
> > > postinstall
> > >     > > cdn
> > >     > > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
> > >     > > >>        70d8893335b63db006974932341378717c42701c: Changing back
> > > the
> > >     > > default
> > >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> > >     > > >> PR 2812: remove traffic_monitor_java
> > >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> > >     > > traffic_monitor_java
> > >     > > >> (Dan Kirkwood, Sep 12 2018)
> > >     > > >>
> > >     > > >>
> > >     > > >> [truncated]
> > >     > > >>
> > >     > > >>
> > >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org>
> > > wrote:
> > >     > > >>>
> > >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > > information on what
> > >     > > >>> actually went into the release, which therefore makes it not
> > > useful.
> > >     > > If we
> > >     > > >>> were better about commit messages and used squash and merge,
> > > then we
> > >     > > would
> > >     > > >>> at least have something for every PR that was merged.
> > >     > > >>>
> > >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com>
> > > wrote:
> > >     > > >>>
> > >     > > >>>>> I think this just shifts the burden from writing a changelog
> > > entry to
> > >     > > >>>> writing a good commit entry.
> > >     > > >>>>
> > >     > > >>>>
> > >     > > >>>> I agree, and I think that’s where that burden belongs. (And I
> > > _really_
> > >     > > >>>> hate merge conflicts).
> > >     > > >>>>
> > >     > > >>>>> There might be fewer commit entries if we squash and merge,
> > > but I’m
> > >     > > >>>> doubtful that it would be as valuable as our “curated”
> > > changelogs.
> > >     > > >>>>
> > >     > > >>>> Both are dependent on how disciplined we are.
> > >     > > >>>>
> > >     > > >>>> I’m +1 on Squash and Merge.
> > >     > > >>>>
> > >     > > >>>> Cheers,
> > >     > > >>>> JvD
> > >     > > >>>>
> > >     > > >>>>
> > >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> > >     > > >>>>>
> > >     > > >>>>> I think this just shifts the burden from writing a changelog
> > > entry to
> > >     > > >>>> writing a good commit entry.
> > >     > > >>>>>
> > >     > > >>>>> There might be fewer commit entries if we squash and merge,
> > > but I’m
> > >     > > >>>> doubtful that it would be as valuable as our “curated”
> > > changelogs.
> > >     > > >>>>>
> > >     > > >>>>>
> > >     > > >>>>>
> > >     > > >>>>>
> > >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org>
> > > wrote:
> > >     > > >>>>>>
> > >     > > >>>>>> Hey All,
> > >     > > >>>>>> I want to follow up on something that was briefly discussed
> > > at the
> > >     > > >>>> summit
> > >     > > >>>>>> this week.  Many people do not like the Changelog and feel
> > > like it
> > >     > > can
> > >     > > >>>> be a
> > >     > > >>>>>> PITA to deal with.  One of the reasons we have the
> > > changelog is
> > >     > > >>> because
> > >     > > >>>>>> there are so many commits in a given release, that it is
> > > hard to
> > >     > > comb
> > >     > > >>>>>> through all of them to figure out what noteworthy changes
> > > or bug
> > >     > > fixes
> > >     > > >>>> went
> > >     > > >>>>>> into the code.  One thing that may help with this problem
> > > is to use
> > >     > > >>>> enable
> > >     > > >>>>>> the squash and merge feature on Github for pull requests
> > > [1].   This
> > >     > > >>>>>> feature would squash all commits in a PR into one commit.
> > > If we
> > >     > > pair
> > >     > > >>>> the
> > >     > > >>>>>> one commit with a good commit message, we would essentially
> > > get the
> > >     > > >>>> ability
> > >     > > >>>>>> to create the changelog just from the commits.
> > >     > > >>>>>>
> > >     > > >>>>>> So, I would like to propose that we enable the squash and
> > > merge
> > >     > > >>> feature
> > >     > > >>>> in
> > >     > > >>>>>> the Traffic Control Github repo and use that going forward.
> > >     > > Thoughts?
> > >     > > >>>>>>
> > >     > > >>>>>> Thanks,
> > >     > > >>>>>> Dave
> > >     > > >>>>>>
> > >     > > >>>>>>
> > >     > > >>>>>> [1]
> > > https://help.github.com/articles/about-pull-request-merges/
> > >     > > >>>>>
> > >     > > >>>>
> > >     > > >>>>
> > >     > > >>>
> > >     > >
> > >
> > >
> > >

Re: [EXTERNAL] Re: Squash and Merge for PRs

["Fieck, Brennan" <Br...@comcast.com>]

This is exactly what I was trying to say, but far more eloquent. +1 for atomic commits
________________________________________
From: Chris Lemmons <al...@gmail.com>
Sent: Tuesday, October 23, 2018 4:49 PM
To: dev@trafficcontrol.apache.org
Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs

I don't think I'm quite -1 on this, but I think we can do better.

I think a PR should be a single unit of work and a commit should be a
single unit of code.

I like using atomic commits. That is, each commit does one thing. So
if I need to revert a fix or track down when something was introduced,
I can find it. For commit messages, I really like the seven rules
documented here, but especially the last one:
https://chris.beams.io/posts/git-commit/

> Use the body to explain what and why vs. how

I think it's important to put the body in the commit message in order
to explain why the change is necessary what the non-obvious effects of
it are. Having a good subject for the commit is also very useful,
since that's what GitHub (and really all the tools) shows when you
scroll through commits, looking for what changed.

In order to have atomic commits, sometimes you'll need one or more
commits as part of a single PR. Not most of the time, but sometimes.
Especially with a large feature, you may make multiple dependent
changes, but each one is a logical unit of work. For example, if the
task is "add support for foobaz plugin", there might be three commits,
with summaries that look like "Add support for foobaz plugin to config
generation", "Add UI for editing foobaz plugin config", and "Add API
endpoints for foobaz plugin config". That also makes it easy to direct
domain experts to each of the changesets for focused review.

A PR on the other hand, is a single unit of work. While each commit
should be stand-alone, capable of running correctly before and after,
each commit might not be desirable on a standalone basis. For example,
while "Add support for foobaz plugin to config generation" is a single
unit of code in that it operates correctly, has tests, is documented,
and is completely implemented, it's not exceptionally useful without a
UI to edit it. It makes sense to group all these changes and merge
them as a unit.

For almost all bug fixes and most medium to small features, each PR
will have precisely one commit. But especially for larger "projects",
it makes the most sense to group it all as one PR, but merge it as a
feature.

Relatedly, it's very rarely correct to merge commits from two
different authors. (This is important for legal reasons as well, if it
ever came to it; authors license their code when they contribute it,
so it's important to know whence it came.) However, in the event that
two people jointly produce what amounts to a single unit of code (for
example, by pair programming, or by collaborating on a snippet of code
on Slack or IRC), you can document that in git by adding it to the end
of the commit message body:

> Co-authored-by: name <na...@example.com>
> Co-authored-by: another-name <an...@example.com>"

Details for that technique here:
https://help.github.com/articles/creating-a-commit-with-multiple-authors/

It's not quite a "standard", but it's quickly being adopted by most of
the tools I use, at least.

As for the "Squash" option, I actually prefer the "Rebase" choice.
It's the same thing if you have single-commit PRs, but it handles
multi-commit PRs a bit more nicely.

So, to summarize, here's what I think:

 - Each commit should be an atomic unit of code and be described by a
descriptive commit subject and body.
 - Each PR should be a logical grouping of closely related work;
usually a single commit.
 - We should use Rebase for flattening our changes.
 - Squash is still an "okay" heuristic that accomplishes the same
thing, most of the time.
On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <de...@apache.org> wrote:
>
> +1 based upon the fact that the Changelog.md can't seem to stay current, we
> do need to be able to maybe at least generate one from the commit messages
> within that release window.
>
> -Dew
>
> On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <Jo...@comcast.com>
> wrote:
>
> > +1
> >
> >  In a past life when using Microsoft VSTS with git, the default action was
> > squash.  The rule of thumb we used was squash by default and only merge if
> > every commit was meaningful and required to understand what you were trying
> > to do.  We didn't typically have communal feature branches, so it wasn't as
> > large a use case for us.  The repo forensics are nice with merge, but the
> > reality we found was that git blame was more useful when we could track to
> > the PR rather than a mid-stream commit and then having to make a second
> > jump to the PR from there.  It also cut down on back/forth design change
> > commits and helped us stay on track with 1 PR == 1 issue because it made
> > unrelated fixes more apparent.
> >
> > Jonathan G
> >
> >
> > On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com>
> > wrote:
> >
> >     I'm not gonna say +1 or -1, but I'd still like to chime in. I think
> > it'd be nice to require PRs to squash things into atomic chunks. No commits
> > that say "started x" or "minor fixes", but idk if it's reasonable to be
> > able to boil every PR forever down to 70 characters. And sure you can make
> > multiple PRs if changes are too big to fit in one commit message, but then
> > it becomes really easy to become entwined in a chain of dependent PRs so
> > that every time one of them gets merged you have to rebase them all.
> >
> >     Of course, a "reasonable squash" is pretty hard to define absolutely,
> > and could actually just make PRs harder to review -  maybe it's not even
> > enforceable at all.
> >     ________________________________________
> >     From: Rawlin Peters <ra...@gmail.com>
> >     Sent: Tuesday, October 23, 2018 11:14 AM
> >     To: dev@trafficcontrol.apache.org
> >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> >
> >     +1 on enabling squash and merge.
> >
> >     This will not only make cherry picks easier to manage with respect to
> >     merge conflicts, but it will also make the git log more useful because
> >     we will be able to see the full changeset of the PR in a single commit
> >     of the git log. Ideally, I'd also like the squashed commit to contain
> >     a link to the actual github PR so that I can easily view comments, but
> >     that is a proposal for another day.
> >
> >     - Rawlin
> >
> >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org>
> > wrote:
> >     >
> >     > This seems to have stalled, so let me try to move it along.
> >     >
> >     > There is an ability to do "squash and merge" in github, we just need
> > to
> >     > agree that we want to enable it for our project.
> >     >
> >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how
> > to
> >     > handle the multiple committers thing, but I assume we can squash
> > into two
> >     > commits and give one to each.  However, if we are doing something big
> >     > enough to require that many commits from multiple users, it may be
> > best as
> >     > many PRs?  I don't think this is a very common situation, so I would
> > like
> >     > to avoid getting bogged down in what-ifs if possible.
> >     >
> >     > As for the Changelog, I am fine with enabling squash and merge and
> > keeping
> >     > the changelog as-is while we decide if squash and merge meets our
> > needs.  I
> >     > am also game to revisit different ways to do it, and I shouldn't have
> >     > conflated that conversation with this one.  If someone wants to see
> > that
> >     > changed, we should start a new discussion.
> >     >
> >     > Leif brings up a good point about squash and merge making cherry
> > picks
> >     > easier so I think we should consider it for that point alone.
> >     >
> >     > So, what I am really asking is:  "do we think it is a good idea to
> > enable
> >     > squash and merge on our Github repo so that we can reduce the amount
> > of
> >     > commits per PR?"  I would like to get consensus for or against.  If
> > it is
> >     > not clear already, I am for.
> >     >
> >     >
> >     > Thanks,
> >     > Dave
> >     >
> >     >
> >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > Derek_Gelinas@comcast.com>
> >     > wrote:
> >     >
> >     > > I'll be honest, I think the simplest thing is still to just
> > require a
> >     > > changelog message, added to a central file, to be included in the
> > commit.
> >     > > We've been going over and over this for months instead of actually
> > writing
> >     > > change logs.  Let's just propose it and put it to a vote.
> >     > >
> >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > rawlin.peters@gmail.com>
> >     > > wrote:
> >     > > >
> >     > > > I'm +1 on squashing and merging, but I think we need a
> > combination of
> >     > > > a number of things to really have a decent changelog. Even if we
> > have
> >     > > > awesome commit messages and squashed PRs, there's still way too
> > many
> >     > > > PRs in a release to just generate a decent changelog from the PR
> >     > > > titles and/or commit messages. We still need a way to logically
> >     > > > separate PRs into bugfixes, new features, breaking changes, etc.
> > We
> >     > > > could get some of that separation using github tags, but what if
> > there
> >     > > > are multiple PRs that compose one new high-level feature? At that
> >     > > > point I think we'd have to create a new tag for every new
> > high-level
> >     > > > feature being worked on in order to group those PRs under the
> > same
> >     > > > heading. There's also the problem that only committers can tag
> > issues,
> >     > > > so it adds to the burden of committers to make sure all
> > non-committer
> >     > > > issues/PRs are properly tagged.
> >     > > >
> >     > > > The main headache with doing a manually-curated changelog is the
> > merge
> >     > > > conflicts it causes, but I think we can easily avoid that
> > problem by
> >     > > > using a similar approach to reno [1] which breaks individual
> > notes out
> >     > > > into their own separate yaml files which are then parsed and
> > merged
> >     > > > into a single, cohesive changelog for each release. What that
> > means is
> >     > > > basically every logical bugfix/new feature/upgrade note gets its
> > own
> >     > > > file which avoids merge conflicts caused by just adding a line
> > to the
> >     > > > end of a CHANGELOG.md file every time.
> >     > > >
> >     > > > - Rawlin
> >     > > >
> >     > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > mitchell852@gmail.com>
> >     > > wrote:
> >     > > >>
> >     > > >> is there a setting in github to enable "squash and merge" vs
> > "rebase and
> >     > > >> merge"?
> >     > > >>
> >     > > >> i do have a couple of concerns however:
> >     > > >>
> >     > > >> 1. what happens when 2+ people contribute to a PR? This is a
> > pretty
> >     > > common
> >     > > >> scenario. If Bob and Sam both work on PR#1 is all the commit
> > history for
> >     > > >> Sam lost?
> >     > > >> 2. i can see benefits of having granular history...to a point...
> >     > > >>
> >     > > >> The more I think about this. Why is squash needed? Commits are
> > grouped
> >     > > by
> >     > > >> PRs. Why isn't our change log just a list of PR merged since
> > the last
> >     > > >> release? If a PR represents a working unit (as it should), PR
> > titles
> >     > > should
> >     > > >> be sufficient, no?
> >     > > >>
> >     > > >> For example, this is what our internal release change log looks
> > like.
> >     > > >>
> >     > > >> Changelog from 2526569a to 284555703
> >     > > >> ========================================
> >     > > >>
> >     > > >> These changes are deployed in branch deploy-20181002.
> >     > > >>
> >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> >     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
> >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> >     > > cachegroups/dses
> >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO
> > unused func
> >     > > >> (Robert Butts, Sep 13 2018)
> >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API
> > Tests for
> >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go
> > client
> >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> >     > > >> PR 2806: Add sub to validate user input when running
> > postinstall script
> >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to
> > validate
> >     > > user
> >     > > >> input when running postinstall script changes default cdn name
> > to not
> >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering
> > postinstall
> >     > > cdn
> >     > > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
> >     > > >>        70d8893335b63db006974932341378717c42701c: Changing back
> > the
> >     > > default
> >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> >     > > >> PR 2812: remove traffic_monitor_java
> >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> >     > > traffic_monitor_java
> >     > > >> (Dan Kirkwood, Sep 12 2018)
> >     > > >>
> >     > > >>
> >     > > >> [truncated]
> >     > > >>
> >     > > >>
> >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org>
> > wrote:
> >     > > >>>
> >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > information on what
> >     > > >>> actually went into the release, which therefore makes it not
> > useful.
> >     > > If we
> >     > > >>> were better about commit messages and used squash and merge,
> > then we
> >     > > would
> >     > > >>> at least have something for every PR that was merged.
> >     > > >>>
> >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com>
> > wrote:
> >     > > >>>
> >     > > >>>>> I think this just shifts the burden from writing a changelog
> > entry to
> >     > > >>>> writing a good commit entry.
> >     > > >>>>
> >     > > >>>>
> >     > > >>>> I agree, and I think that’s where that burden belongs. (And I
> > _really_
> >     > > >>>> hate merge conflicts).
> >     > > >>>>
> >     > > >>>>> There might be fewer commit entries if we squash and merge,
> > but I’m
> >     > > >>>> doubtful that it would be as valuable as our “curated”
> > changelogs.
> >     > > >>>>
> >     > > >>>> Both are dependent on how disciplined we are.
> >     > > >>>>
> >     > > >>>> I’m +1 on Squash and Merge.
> >     > > >>>>
> >     > > >>>> Cheers,
> >     > > >>>> JvD
> >     > > >>>>
> >     > > >>>>
> >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> >     > > >>>>>
> >     > > >>>>> I think this just shifts the burden from writing a changelog
> > entry to
> >     > > >>>> writing a good commit entry.
> >     > > >>>>>
> >     > > >>>>> There might be fewer commit entries if we squash and merge,
> > but I’m
> >     > > >>>> doubtful that it would be as valuable as our “curated”
> > changelogs.
> >     > > >>>>>
> >     > > >>>>>
> >     > > >>>>>
> >     > > >>>>>
> >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org>
> > wrote:
> >     > > >>>>>>
> >     > > >>>>>> Hey All,
> >     > > >>>>>> I want to follow up on something that was briefly discussed
> > at the
> >     > > >>>> summit
> >     > > >>>>>> this week.  Many people do not like the Changelog and feel
> > like it
> >     > > can
> >     > > >>>> be a
> >     > > >>>>>> PITA to deal with.  One of the reasons we have the
> > changelog is
> >     > > >>> because
> >     > > >>>>>> there are so many commits in a given release, that it is
> > hard to
> >     > > comb
> >     > > >>>>>> through all of them to figure out what noteworthy changes
> > or bug
> >     > > fixes
> >     > > >>>> went
> >     > > >>>>>> into the code.  One thing that may help with this problem
> > is to use
> >     > > >>>> enable
> >     > > >>>>>> the squash and merge feature on Github for pull requests
> > [1].   This
> >     > > >>>>>> feature would squash all commits in a PR into one commit.
> > If we
> >     > > pair
> >     > > >>>> the
> >     > > >>>>>> one commit with a good commit message, we would essentially
> > get the
> >     > > >>>> ability
> >     > > >>>>>> to create the changelog just from the commits.
> >     > > >>>>>>
> >     > > >>>>>> So, I would like to propose that we enable the squash and
> > merge
> >     > > >>> feature
> >     > > >>>> in
> >     > > >>>>>> the Traffic Control Github repo and use that going forward.
> >     > > Thoughts?
> >     > > >>>>>>
> >     > > >>>>>> Thanks,
> >     > > >>>>>> Dave
> >     > > >>>>>>
> >     > > >>>>>>
> >     > > >>>>>> [1]
> > https://help.github.com/articles/about-pull-request-merges/
> >     > > >>>>>
> >     > > >>>>
> >     > > >>>>
> >     > > >>>
> >     > >
> >
> >
> >

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Chris Lemmons <al...@gmail.com>]

I don't think I'm quite -1 on this, but I think we can do better.

I think a PR should be a single unit of work and a commit should be a
single unit of code.

I like using atomic commits. That is, each commit does one thing. So
if I need to revert a fix or track down when something was introduced,
I can find it. For commit messages, I really like the seven rules
documented here, but especially the last one:
https://chris.beams.io/posts/git-commit/

> Use the body to explain what and why vs. how

I think it's important to put the body in the commit message in order
to explain why the change is necessary what the non-obvious effects of
it are. Having a good subject for the commit is also very useful,
since that's what GitHub (and really all the tools) shows when you
scroll through commits, looking for what changed.

In order to have atomic commits, sometimes you'll need one or more
commits as part of a single PR. Not most of the time, but sometimes.
Especially with a large feature, you may make multiple dependent
changes, but each one is a logical unit of work. For example, if the
task is "add support for foobaz plugin", there might be three commits,
with summaries that look like "Add support for foobaz plugin to config
generation", "Add UI for editing foobaz plugin config", and "Add API
endpoints for foobaz plugin config". That also makes it easy to direct
domain experts to each of the changesets for focused review.

A PR on the other hand, is a single unit of work. While each commit
should be stand-alone, capable of running correctly before and after,
each commit might not be desirable on a standalone basis. For example,
while "Add support for foobaz plugin to config generation" is a single
unit of code in that it operates correctly, has tests, is documented,
and is completely implemented, it's not exceptionally useful without a
UI to edit it. It makes sense to group all these changes and merge
them as a unit.

For almost all bug fixes and most medium to small features, each PR
will have precisely one commit. But especially for larger "projects",
it makes the most sense to group it all as one PR, but merge it as a
feature.

Relatedly, it's very rarely correct to merge commits from two
different authors. (This is important for legal reasons as well, if it
ever came to it; authors license their code when they contribute it,
so it's important to know whence it came.) However, in the event that
two people jointly produce what amounts to a single unit of code (for
example, by pair programming, or by collaborating on a snippet of code
on Slack or IRC), you can document that in git by adding it to the end
of the commit message body:

> Co-authored-by: name <na...@example.com>
> Co-authored-by: another-name <an...@example.com>"

Details for that technique here:
https://help.github.com/articles/creating-a-commit-with-multiple-authors/

It's not quite a "standard", but it's quickly being adopted by most of
the tools I use, at least.

As for the "Squash" option, I actually prefer the "Rebase" choice.
It's the same thing if you have single-commit PRs, but it handles
multi-commit PRs a bit more nicely.

So, to summarize, here's what I think:

 - Each commit should be an atomic unit of code and be described by a
descriptive commit subject and body.
 - Each PR should be a logical grouping of closely related work;
usually a single commit.
 - We should use Rebase for flattening our changes.
 - Squash is still an "okay" heuristic that accomplishes the same
thing, most of the time.
On Tue, Oct 23, 2018 at 1:06 PM Dewayne Richardson <de...@apache.org> wrote:
>
> +1 based upon the fact that the Changelog.md can't seem to stay current, we
> do need to be able to maybe at least generate one from the commit messages
> within that release window.
>
> -Dew
>
> On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <Jo...@comcast.com>
> wrote:
>
> > +1
> >
> >  In a past life when using Microsoft VSTS with git, the default action was
> > squash.  The rule of thumb we used was squash by default and only merge if
> > every commit was meaningful and required to understand what you were trying
> > to do.  We didn't typically have communal feature branches, so it wasn't as
> > large a use case for us.  The repo forensics are nice with merge, but the
> > reality we found was that git blame was more useful when we could track to
> > the PR rather than a mid-stream commit and then having to make a second
> > jump to the PR from there.  It also cut down on back/forth design change
> > commits and helped us stay on track with 1 PR == 1 issue because it made
> > unrelated fixes more apparent.
> >
> > Jonathan G
> >
> >
> > On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com>
> > wrote:
> >
> >     I'm not gonna say +1 or -1, but I'd still like to chime in. I think
> > it'd be nice to require PRs to squash things into atomic chunks. No commits
> > that say "started x" or "minor fixes", but idk if it's reasonable to be
> > able to boil every PR forever down to 70 characters. And sure you can make
> > multiple PRs if changes are too big to fit in one commit message, but then
> > it becomes really easy to become entwined in a chain of dependent PRs so
> > that every time one of them gets merged you have to rebase them all.
> >
> >     Of course, a "reasonable squash" is pretty hard to define absolutely,
> > and could actually just make PRs harder to review -  maybe it's not even
> > enforceable at all.
> >     ________________________________________
> >     From: Rawlin Peters <ra...@gmail.com>
> >     Sent: Tuesday, October 23, 2018 11:14 AM
> >     To: dev@trafficcontrol.apache.org
> >     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
> >
> >     +1 on enabling squash and merge.
> >
> >     This will not only make cherry picks easier to manage with respect to
> >     merge conflicts, but it will also make the git log more useful because
> >     we will be able to see the full changeset of the PR in a single commit
> >     of the git log. Ideally, I'd also like the squashed commit to contain
> >     a link to the actual github PR so that I can easily view comments, but
> >     that is a proposal for another day.
> >
> >     - Rawlin
> >
> >     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org>
> > wrote:
> >     >
> >     > This seems to have stalled, so let me try to move it along.
> >     >
> >     > There is an ability to do "squash and merge" in github, we just need
> > to
> >     > agree that we want to enable it for our project.
> >     >
> >     > @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how
> > to
> >     > handle the multiple committers thing, but I assume we can squash
> > into two
> >     > commits and give one to each.  However, if we are doing something big
> >     > enough to require that many commits from multiple users, it may be
> > best as
> >     > many PRs?  I don't think this is a very common situation, so I would
> > like
> >     > to avoid getting bogged down in what-ifs if possible.
> >     >
> >     > As for the Changelog, I am fine with enabling squash and merge and
> > keeping
> >     > the changelog as-is while we decide if squash and merge meets our
> > needs.  I
> >     > am also game to revisit different ways to do it, and I shouldn't have
> >     > conflated that conversation with this one.  If someone wants to see
> > that
> >     > changed, we should start a new discussion.
> >     >
> >     > Leif brings up a good point about squash and merge making cherry
> > picks
> >     > easier so I think we should consider it for that point alone.
> >     >
> >     > So, what I am really asking is:  "do we think it is a good idea to
> > enable
> >     > squash and merge on our Github repo so that we can reduce the amount
> > of
> >     > commits per PR?"  I would like to get consensus for or against.  If
> > it is
> >     > not clear already, I am for.
> >     >
> >     >
> >     > Thanks,
> >     > Dave
> >     >
> >     >
> >     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> > Derek_Gelinas@comcast.com>
> >     > wrote:
> >     >
> >     > > I'll be honest, I think the simplest thing is still to just
> > require a
> >     > > changelog message, added to a central file, to be included in the
> > commit.
> >     > > We've been going over and over this for months instead of actually
> > writing
> >     > > change logs.  Let's just propose it and put it to a vote.
> >     > >
> >     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> > rawlin.peters@gmail.com>
> >     > > wrote:
> >     > > >
> >     > > > I'm +1 on squashing and merging, but I think we need a
> > combination of
> >     > > > a number of things to really have a decent changelog. Even if we
> > have
> >     > > > awesome commit messages and squashed PRs, there's still way too
> > many
> >     > > > PRs in a release to just generate a decent changelog from the PR
> >     > > > titles and/or commit messages. We still need a way to logically
> >     > > > separate PRs into bugfixes, new features, breaking changes, etc.
> > We
> >     > > > could get some of that separation using github tags, but what if
> > there
> >     > > > are multiple PRs that compose one new high-level feature? At that
> >     > > > point I think we'd have to create a new tag for every new
> > high-level
> >     > > > feature being worked on in order to group those PRs under the
> > same
> >     > > > heading. There's also the problem that only committers can tag
> > issues,
> >     > > > so it adds to the burden of committers to make sure all
> > non-committer
> >     > > > issues/PRs are properly tagged.
> >     > > >
> >     > > > The main headache with doing a manually-curated changelog is the
> > merge
> >     > > > conflicts it causes, but I think we can easily avoid that
> > problem by
> >     > > > using a similar approach to reno [1] which breaks individual
> > notes out
> >     > > > into their own separate yaml files which are then parsed and
> > merged
> >     > > > into a single, cohesive changelog for each release. What that
> > means is
> >     > > > basically every logical bugfix/new feature/upgrade note gets its
> > own
> >     > > > file which avoids merge conflicts caused by just adding a line
> > to the
> >     > > > end of a CHANGELOG.md file every time.
> >     > > >
> >     > > > - Rawlin
> >     > > >
> >     > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> >     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> > mitchell852@gmail.com>
> >     > > wrote:
> >     > > >>
> >     > > >> is there a setting in github to enable "squash and merge" vs
> > "rebase and
> >     > > >> merge"?
> >     > > >>
> >     > > >> i do have a couple of concerns however:
> >     > > >>
> >     > > >> 1. what happens when 2+ people contribute to a PR? This is a
> > pretty
> >     > > common
> >     > > >> scenario. If Bob and Sam both work on PR#1 is all the commit
> > history for
> >     > > >> Sam lost?
> >     > > >> 2. i can see benefits of having granular history...to a point...
> >     > > >>
> >     > > >> The more I think about this. Why is squash needed? Commits are
> > grouped
> >     > > by
> >     > > >> PRs. Why isn't our change log just a list of PR merged since
> > the last
> >     > > >> release? If a PR represents a working unit (as it should), PR
> > titles
> >     > > should
> >     > > >> be sufficient, no?
> >     > > >>
> >     > > >> For example, this is what our internal release change log looks
> > like.
> >     > > >>
> >     > > >> Changelog from 2526569a to 284555703
> >     > > >> ========================================
> >     > > >>
> >     > > >> These changes are deployed in branch deploy-20181002.
> >     > > >>
> >     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> >     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> >     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
> >     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> >     > > cachegroups/dses
> >     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
> >     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO
> > unused func
> >     > > >> (Robert Butts, Sep 13 2018)
> >     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
> >     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> >     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
> >     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API
> > Tests for
> >     > > >> steeringtargets (Robert Butts, Sep 11 2018)
> >     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go
> > client
> >     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> >     > > >> PR 2806: Add sub to validate user input when running
> > postinstall script
> >     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to
> > validate
> >     > > user
> >     > > >> input when running postinstall script changes default cdn name
> > to not
> >     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
> >     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering
> > postinstall
> >     > > cdn
> >     > > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
> >     > > >>        70d8893335b63db006974932341378717c42701c: Changing back
> > the
> >     > > default
> >     > > >> cdn name value (Dylan Souza, Sep 21 2018)
> >     > > >> PR 2812: remove traffic_monitor_java
> >     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> >     > > traffic_monitor_java
> >     > > >> (Dan Kirkwood, Sep 12 2018)
> >     > > >>
> >     > > >>
> >     > > >> [truncated]
> >     > > >>
> >     > > >>
> >     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org>
> > wrote:
> >     > > >>>
> >     > > >>> Well, our "curated" changelogs are missing A LOT of
> > information on what
> >     > > >>> actually went into the release, which therefore makes it not
> > useful.
> >     > > If we
> >     > > >>> were better about commit messages and used squash and merge,
> > then we
> >     > > would
> >     > > >>> at least have something for every PR that was merged.
> >     > > >>>
> >     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com>
> > wrote:
> >     > > >>>
> >     > > >>>>> I think this just shifts the burden from writing a changelog
> > entry to
> >     > > >>>> writing a good commit entry.
> >     > > >>>>
> >     > > >>>>
> >     > > >>>> I agree, and I think that’s where that burden belongs. (And I
> > _really_
> >     > > >>>> hate merge conflicts).
> >     > > >>>>
> >     > > >>>>> There might be fewer commit entries if we squash and merge,
> > but I’m
> >     > > >>>> doubtful that it would be as valuable as our “curated”
> > changelogs.
> >     > > >>>>
> >     > > >>>> Both are dependent on how disciplined we are.
> >     > > >>>>
> >     > > >>>> I’m +1 on Squash and Merge.
> >     > > >>>>
> >     > > >>>> Cheers,
> >     > > >>>> JvD
> >     > > >>>>
> >     > > >>>>
> >     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> >     > > >>>> <ef...@cisco.com.INVALID> wrote:
> >     > > >>>>>
> >     > > >>>>> I think this just shifts the burden from writing a changelog
> > entry to
> >     > > >>>> writing a good commit entry.
> >     > > >>>>>
> >     > > >>>>> There might be fewer commit entries if we squash and merge,
> > but I’m
> >     > > >>>> doubtful that it would be as valuable as our “curated”
> > changelogs.
> >     > > >>>>>
> >     > > >>>>>
> >     > > >>>>>
> >     > > >>>>>
> >     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org>
> > wrote:
> >     > > >>>>>>
> >     > > >>>>>> Hey All,
> >     > > >>>>>> I want to follow up on something that was briefly discussed
> > at the
> >     > > >>>> summit
> >     > > >>>>>> this week.  Many people do not like the Changelog and feel
> > like it
> >     > > can
> >     > > >>>> be a
> >     > > >>>>>> PITA to deal with.  One of the reasons we have the
> > changelog is
> >     > > >>> because
> >     > > >>>>>> there are so many commits in a given release, that it is
> > hard to
> >     > > comb
> >     > > >>>>>> through all of them to figure out what noteworthy changes
> > or bug
> >     > > fixes
> >     > > >>>> went
> >     > > >>>>>> into the code.  One thing that may help with this problem
> > is to use
> >     > > >>>> enable
> >     > > >>>>>> the squash and merge feature on Github for pull requests
> > [1].   This
> >     > > >>>>>> feature would squash all commits in a PR into one commit.
> > If we
> >     > > pair
> >     > > >>>> the
> >     > > >>>>>> one commit with a good commit message, we would essentially
> > get the
> >     > > >>>> ability
> >     > > >>>>>> to create the changelog just from the commits.
> >     > > >>>>>>
> >     > > >>>>>> So, I would like to propose that we enable the squash and
> > merge
> >     > > >>> feature
> >     > > >>>> in
> >     > > >>>>>> the Traffic Control Github repo and use that going forward.
> >     > > Thoughts?
> >     > > >>>>>>
> >     > > >>>>>> Thanks,
> >     > > >>>>>> Dave
> >     > > >>>>>>
> >     > > >>>>>>
> >     > > >>>>>> [1]
> > https://help.github.com/articles/about-pull-request-merges/
> >     > > >>>>>
> >     > > >>>>
> >     > > >>>>
> >     > > >>>
> >     > >
> >
> >
> >

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Dewayne Richardson <de...@apache.org>]

+1 based upon the fact that the Changelog.md can't seem to stay current, we
do need to be able to maybe at least generate one from the commit messages
within that release window.

-Dew

On Tue, Oct 23, 2018 at 12:58 PM Gray, Jonathan <Jo...@comcast.com>
wrote:

> +1
>
>  In a past life when using Microsoft VSTS with git, the default action was
> squash.  The rule of thumb we used was squash by default and only merge if
> every commit was meaningful and required to understand what you were trying
> to do.  We didn't typically have communal feature branches, so it wasn't as
> large a use case for us.  The repo forensics are nice with merge, but the
> reality we found was that git blame was more useful when we could track to
> the PR rather than a mid-stream commit and then having to make a second
> jump to the PR from there.  It also cut down on back/forth design change
> commits and helped us stay on track with 1 PR == 1 issue because it made
> unrelated fixes more apparent.
>
> Jonathan G
>
>
> On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com>
> wrote:
>
>     I'm not gonna say +1 or -1, but I'd still like to chime in. I think
> it'd be nice to require PRs to squash things into atomic chunks. No commits
> that say "started x" or "minor fixes", but idk if it's reasonable to be
> able to boil every PR forever down to 70 characters. And sure you can make
> multiple PRs if changes are too big to fit in one commit message, but then
> it becomes really easy to become entwined in a chain of dependent PRs so
> that every time one of them gets merged you have to rebase them all.
>
>     Of course, a "reasonable squash" is pretty hard to define absolutely,
> and could actually just make PRs harder to review -  maybe it's not even
> enforceable at all.
>     ________________________________________
>     From: Rawlin Peters <ra...@gmail.com>
>     Sent: Tuesday, October 23, 2018 11:14 AM
>     To: dev@trafficcontrol.apache.org
>     Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
>
>     +1 on enabling squash and merge.
>
>     This will not only make cherry picks easier to manage with respect to
>     merge conflicts, but it will also make the git log more useful because
>     we will be able to see the full changeset of the PR in a single commit
>     of the git log. Ideally, I'd also like the squashed commit to contain
>     a link to the actual github PR so that I can easily view comments, but
>     that is a proposal for another day.
>
>     - Rawlin
>
>     On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org>
> wrote:
>     >
>     > This seems to have stalled, so let me try to move it along.
>     >
>     > There is an ability to do "squash and merge" in github, we just need
> to
>     > agree that we want to enable it for our project.
>     >
>     > @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how
> to
>     > handle the multiple committers thing, but I assume we can squash
> into two
>     > commits and give one to each.  However, if we are doing something big
>     > enough to require that many commits from multiple users, it may be
> best as
>     > many PRs?  I don't think this is a very common situation, so I would
> like
>     > to avoid getting bogged down in what-ifs if possible.
>     >
>     > As for the Changelog, I am fine with enabling squash and merge and
> keeping
>     > the changelog as-is while we decide if squash and merge meets our
> needs.  I
>     > am also game to revisit different ways to do it, and I shouldn't have
>     > conflated that conversation with this one.  If someone wants to see
> that
>     > changed, we should start a new discussion.
>     >
>     > Leif brings up a good point about squash and merge making cherry
> picks
>     > easier so I think we should consider it for that point alone.
>     >
>     > So, what I am really asking is:  "do we think it is a good idea to
> enable
>     > squash and merge on our Github repo so that we can reduce the amount
> of
>     > commits per PR?"  I would like to get consensus for or against.  If
> it is
>     > not clear already, I am for.
>     >
>     >
>     > Thanks,
>     > Dave
>     >
>     >
>     > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <
> Derek_Gelinas@comcast.com>
>     > wrote:
>     >
>     > > I'll be honest, I think the simplest thing is still to just
> require a
>     > > changelog message, added to a central file, to be included in the
> commit.
>     > > We've been going over and over this for months instead of actually
> writing
>     > > change logs.  Let's just propose it and put it to a vote.
>     > >
>     > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <
> rawlin.peters@gmail.com>
>     > > wrote:
>     > > >
>     > > > I'm +1 on squashing and merging, but I think we need a
> combination of
>     > > > a number of things to really have a decent changelog. Even if we
> have
>     > > > awesome commit messages and squashed PRs, there's still way too
> many
>     > > > PRs in a release to just generate a decent changelog from the PR
>     > > > titles and/or commit messages. We still need a way to logically
>     > > > separate PRs into bugfixes, new features, breaking changes, etc.
> We
>     > > > could get some of that separation using github tags, but what if
> there
>     > > > are multiple PRs that compose one new high-level feature? At that
>     > > > point I think we'd have to create a new tag for every new
> high-level
>     > > > feature being worked on in order to group those PRs under the
> same
>     > > > heading. There's also the problem that only committers can tag
> issues,
>     > > > so it adds to the burden of committers to make sure all
> non-committer
>     > > > issues/PRs are properly tagged.
>     > > >
>     > > > The main headache with doing a manually-curated changelog is the
> merge
>     > > > conflicts it causes, but I think we can easily avoid that
> problem by
>     > > > using a similar approach to reno [1] which breaks individual
> notes out
>     > > > into their own separate yaml files which are then parsed and
> merged
>     > > > into a single, cohesive changelog for each release. What that
> means is
>     > > > basically every logical bugfix/new feature/upgrade note gets its
> own
>     > > > file which avoids merge conflicts caused by just adding a line
> to the
>     > > > end of a CHANGELOG.md file every time.
>     > > >
>     > > > - Rawlin
>     > > >
>     > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
>     > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <
> mitchell852@gmail.com>
>     > > wrote:
>     > > >>
>     > > >> is there a setting in github to enable "squash and merge" vs
> "rebase and
>     > > >> merge"?
>     > > >>
>     > > >> i do have a couple of concerns however:
>     > > >>
>     > > >> 1. what happens when 2+ people contribute to a PR? This is a
> pretty
>     > > common
>     > > >> scenario. If Bob and Sam both work on PR#1 is all the commit
> history for
>     > > >> Sam lost?
>     > > >> 2. i can see benefits of having granular history...to a point...
>     > > >>
>     > > >> The more I think about this. Why is squash needed? Commits are
> grouped
>     > > by
>     > > >> PRs. Why isn't our change log just a list of PR merged since
> the last
>     > > >> release? If a PR represents a working unit (as it should), PR
> titles
>     > > should
>     > > >> be sufficient, no?
>     > > >>
>     > > >> For example, this is what our internal release change log looks
> like.
>     > > >>
>     > > >> Changelog from 2526569a to 284555703
>     > > >> ========================================
>     > > >>
>     > > >> These changes are deployed in branch deploy-20181002.
>     > > >>
>     > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
>     > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
>     > > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
>     > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
>     > > cachegroups/dses
>     > > >> API test, client funcs (Robert Butts, Jun 28 2018)
>     > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO
> unused func
>     > > >> (Robert Butts, Sep 13 2018)
>     > > >> PR 2803: Add Traffic Ops Golang Steering Targets
>     > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
>     > > >> steering/id/targets (Robert Butts, Jul 10 2018)
>     > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API
> Tests for
>     > > >> steeringtargets (Robert Butts, Sep 11 2018)
>     > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go
> client
>     > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
>     > > >> PR 2806: Add sub to validate user input when running
> postinstall script
>     > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to
> validate
>     > > user
>     > > >> input when running postinstall script changes default cdn name
> to not
>     > > >> include an underscore. (Dylan Souza, Sep 11 2018)
>     > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering
> postinstall
>     > > cdn
>     > > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
>     > > >>        70d8893335b63db006974932341378717c42701c: Changing back
> the
>     > > default
>     > > >> cdn name value (Dylan Souza, Sep 21 2018)
>     > > >> PR 2812: remove traffic_monitor_java
>     > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
>     > > traffic_monitor_java
>     > > >> (Dan Kirkwood, Sep 12 2018)
>     > > >>
>     > > >>
>     > > >> [truncated]
>     > > >>
>     > > >>
>     > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org>
> wrote:
>     > > >>>
>     > > >>> Well, our "curated" changelogs are missing A LOT of
> information on what
>     > > >>> actually went into the release, which therefore makes it not
> useful.
>     > > If we
>     > > >>> were better about commit messages and used squash and merge,
> then we
>     > > would
>     > > >>> at least have something for every PR that was merged.
>     > > >>>
>     > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com>
> wrote:
>     > > >>>
>     > > >>>>> I think this just shifts the burden from writing a changelog
> entry to
>     > > >>>> writing a good commit entry.
>     > > >>>>
>     > > >>>>
>     > > >>>> I agree, and I think that’s where that burden belongs. (And I
> _really_
>     > > >>>> hate merge conflicts).
>     > > >>>>
>     > > >>>>> There might be fewer commit entries if we squash and merge,
> but I’m
>     > > >>>> doubtful that it would be as valuable as our “curated”
> changelogs.
>     > > >>>>
>     > > >>>> Both are dependent on how disciplined we are.
>     > > >>>>
>     > > >>>> I’m +1 on Squash and Merge.
>     > > >>>>
>     > > >>>> Cheers,
>     > > >>>> JvD
>     > > >>>>
>     > > >>>>
>     > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
>     > > >>>> <ef...@cisco.com.INVALID> wrote:
>     > > >>>>>
>     > > >>>>> I think this just shifts the burden from writing a changelog
> entry to
>     > > >>>> writing a good commit entry.
>     > > >>>>>
>     > > >>>>> There might be fewer commit entries if we squash and merge,
> but I’m
>     > > >>>> doubtful that it would be as valuable as our “curated”
> changelogs.
>     > > >>>>>
>     > > >>>>>
>     > > >>>>>
>     > > >>>>>
>     > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org>
> wrote:
>     > > >>>>>>
>     > > >>>>>> Hey All,
>     > > >>>>>> I want to follow up on something that was briefly discussed
> at the
>     > > >>>> summit
>     > > >>>>>> this week.  Many people do not like the Changelog and feel
> like it
>     > > can
>     > > >>>> be a
>     > > >>>>>> PITA to deal with.  One of the reasons we have the
> changelog is
>     > > >>> because
>     > > >>>>>> there are so many commits in a given release, that it is
> hard to
>     > > comb
>     > > >>>>>> through all of them to figure out what noteworthy changes
> or bug
>     > > fixes
>     > > >>>> went
>     > > >>>>>> into the code.  One thing that may help with this problem
> is to use
>     > > >>>> enable
>     > > >>>>>> the squash and merge feature on Github for pull requests
> [1].   This
>     > > >>>>>> feature would squash all commits in a PR into one commit.
> If we
>     > > pair
>     > > >>>> the
>     > > >>>>>> one commit with a good commit message, we would essentially
> get the
>     > > >>>> ability
>     > > >>>>>> to create the changelog just from the commits.
>     > > >>>>>>
>     > > >>>>>> So, I would like to propose that we enable the squash and
> merge
>     > > >>> feature
>     > > >>>> in
>     > > >>>>>> the Traffic Control Github repo and use that going forward.
>     > > Thoughts?
>     > > >>>>>>
>     > > >>>>>> Thanks,
>     > > >>>>>> Dave
>     > > >>>>>>
>     > > >>>>>>
>     > > >>>>>> [1]
> https://help.github.com/articles/about-pull-request-merges/
>     > > >>>>>
>     > > >>>>
>     > > >>>>
>     > > >>>
>     > >
>
>
>

Re: [EXTERNAL] Re: Squash and Merge for PRs

["Gray, Jonathan" <Jo...@comcast.com>]

+1

 In a past life when using Microsoft VSTS with git, the default action was squash.  The rule of thumb we used was squash by default and only merge if every commit was meaningful and required to understand what you were trying to do.  We didn't typically have communal feature branches, so it wasn't as large a use case for us.  The repo forensics are nice with merge, but the reality we found was that git blame was more useful when we could track to the PR rather than a mid-stream commit and then having to make a second jump to the PR from there.  It also cut down on back/forth design change commits and helped us stay on track with 1 PR == 1 issue because it made unrelated fixes more apparent.

Jonathan G


On 10/23/18, 12:40 PM, "Fieck, Brennan" <Br...@comcast.com> wrote:

    I'm not gonna say +1 or -1, but I'd still like to chime in. I think it'd be nice to require PRs to squash things into atomic chunks. No commits that say "started x" or "minor fixes", but idk if it's reasonable to be able to boil every PR forever down to 70 characters. And sure you can make multiple PRs if changes are too big to fit in one commit message, but then it becomes really easy to become entwined in a chain of dependent PRs so that every time one of them gets merged you have to rebase them all.
    
    Of course, a "reasonable squash" is pretty hard to define absolutely, and could actually just make PRs harder to review -  maybe it's not even enforceable at all.
    ________________________________________
    From: Rawlin Peters <ra...@gmail.com>
    Sent: Tuesday, October 23, 2018 11:14 AM
    To: dev@trafficcontrol.apache.org
    Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs
    
    +1 on enabling squash and merge.
    
    This will not only make cherry picks easier to manage with respect to
    merge conflicts, but it will also make the git log more useful because
    we will be able to see the full changeset of the PR in a single commit
    of the git log. Ideally, I'd also like the squashed commit to contain
    a link to the actual github PR so that I can easily view comments, but
    that is a proposal for another day.
    
    - Rawlin
    
    On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org> wrote:
    >
    > This seems to have stalled, so let me try to move it along.
    >
    > There is an ability to do "squash and merge" in github, we just need to
    > agree that we want to enable it for our project.
    >
    > @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how to
    > handle the multiple committers thing, but I assume we can squash into two
    > commits and give one to each.  However, if we are doing something big
    > enough to require that many commits from multiple users, it may be best as
    > many PRs?  I don't think this is a very common situation, so I would like
    > to avoid getting bogged down in what-ifs if possible.
    >
    > As for the Changelog, I am fine with enabling squash and merge and keeping
    > the changelog as-is while we decide if squash and merge meets our needs.  I
    > am also game to revisit different ways to do it, and I shouldn't have
    > conflated that conversation with this one.  If someone wants to see that
    > changed, we should start a new discussion.
    >
    > Leif brings up a good point about squash and merge making cherry picks
    > easier so I think we should consider it for that point alone.
    >
    > So, what I am really asking is:  "do we think it is a good idea to enable
    > squash and merge on our Github repo so that we can reduce the amount of
    > commits per PR?"  I would like to get consensus for or against.  If it is
    > not clear already, I am for.
    >
    >
    > Thanks,
    > Dave
    >
    >
    > On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <De...@comcast.com>
    > wrote:
    >
    > > I'll be honest, I think the simplest thing is still to just require a
    > > changelog message, added to a central file, to be included in the commit.
    > > We've been going over and over this for months instead of actually writing
    > > change logs.  Let's just propose it and put it to a vote.
    > >
    > > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <ra...@gmail.com>
    > > wrote:
    > > >
    > > > I'm +1 on squashing and merging, but I think we need a combination of
    > > > a number of things to really have a decent changelog. Even if we have
    > > > awesome commit messages and squashed PRs, there's still way too many
    > > > PRs in a release to just generate a decent changelog from the PR
    > > > titles and/or commit messages. We still need a way to logically
    > > > separate PRs into bugfixes, new features, breaking changes, etc. We
    > > > could get some of that separation using github tags, but what if there
    > > > are multiple PRs that compose one new high-level feature? At that
    > > > point I think we'd have to create a new tag for every new high-level
    > > > feature being worked on in order to group those PRs under the same
    > > > heading. There's also the problem that only committers can tag issues,
    > > > so it adds to the burden of committers to make sure all non-committer
    > > > issues/PRs are properly tagged.
    > > >
    > > > The main headache with doing a manually-curated changelog is the merge
    > > > conflicts it causes, but I think we can easily avoid that problem by
    > > > using a similar approach to reno [1] which breaks individual notes out
    > > > into their own separate yaml files which are then parsed and merged
    > > > into a single, cohesive changelog for each release. What that means is
    > > > basically every logical bugfix/new feature/upgrade note gets its own
    > > > file which avoids merge conflicts caused by just adding a line to the
    > > > end of a CHANGELOG.md file every time.
    > > >
    > > > - Rawlin
    > > >
    > > > [1] https://docs.openstack.org/reno/latest/user/usage.html
    > > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com>
    > > wrote:
    > > >>
    > > >> is there a setting in github to enable "squash and merge" vs "rebase and
    > > >> merge"?
    > > >>
    > > >> i do have a couple of concerns however:
    > > >>
    > > >> 1. what happens when 2+ people contribute to a PR? This is a pretty
    > > common
    > > >> scenario. If Bob and Sam both work on PR#1 is all the commit history for
    > > >> Sam lost?
    > > >> 2. i can see benefits of having granular history...to a point...
    > > >>
    > > >> The more I think about this. Why is squash needed? Commits are grouped
    > > by
    > > >> PRs. Why isn't our change log just a list of PR merged since the last
    > > >> release? If a PR represents a working unit (as it should), PR titles
    > > should
    > > >> be sufficient, no?
    > > >>
    > > >> For example, this is what our internal release change log looks like.
    > > >>
    > > >> Changelog from 2526569a to 284555703
    > > >> ========================================
    > > >>
    > > >> These changes are deployed in branch deploy-20181002.
    > > >>
    > > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
    > > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
    > > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
    > > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
    > > cachegroups/dses
    > > >> API test, client funcs (Robert Butts, Jun 28 2018)
    > > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
    > > >> (Robert Butts, Sep 13 2018)
    > > >> PR 2803: Add Traffic Ops Golang Steering Targets
    > > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
    > > >> steering/id/targets (Robert Butts, Jul 10 2018)
    > > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
    > > >> steeringtargets (Robert Butts, Sep 11 2018)
    > > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
    > > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
    > > >> PR 2806: Add sub to validate user input when running postinstall script
    > > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate
    > > user
    > > >> input when running postinstall script changes default cdn name to not
    > > >> include an underscore. (Dylan Souza, Sep 11 2018)
    > > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall
    > > cdn
    > > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
    > > >>        70d8893335b63db006974932341378717c42701c: Changing back the
    > > default
    > > >> cdn name value (Dylan Souza, Sep 21 2018)
    > > >> PR 2812: remove traffic_monitor_java
    > > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
    > > traffic_monitor_java
    > > >> (Dan Kirkwood, Sep 12 2018)
    > > >>
    > > >>
    > > >> [truncated]
    > > >>
    > > >>
    > > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
    > > >>>
    > > >>> Well, our "curated" changelogs are missing A LOT of information on what
    > > >>> actually went into the release, which therefore makes it not useful.
    > > If we
    > > >>> were better about commit messages and used squash and merge, then we
    > > would
    > > >>> at least have something for every PR that was merged.
    > > >>>
    > > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
    > > >>>
    > > >>>>> I think this just shifts the burden from writing a changelog entry to
    > > >>>> writing a good commit entry.
    > > >>>>
    > > >>>>
    > > >>>> I agree, and I think that’s where that burden belongs. (And I _really_
    > > >>>> hate merge conflicts).
    > > >>>>
    > > >>>>> There might be fewer commit entries if we squash and merge, but I’m
    > > >>>> doubtful that it would be as valuable as our “curated” changelogs.
    > > >>>>
    > > >>>> Both are dependent on how disciplined we are.
    > > >>>>
    > > >>>> I’m +1 on Squash and Merge.
    > > >>>>
    > > >>>> Cheers,
    > > >>>> JvD
    > > >>>>
    > > >>>>
    > > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
    > > >>>> <ef...@cisco.com.INVALID> wrote:
    > > >>>>>
    > > >>>>> I think this just shifts the burden from writing a changelog entry to
    > > >>>> writing a good commit entry.
    > > >>>>>
    > > >>>>> There might be fewer commit entries if we squash and merge, but I’m
    > > >>>> doubtful that it would be as valuable as our “curated” changelogs.
    > > >>>>>
    > > >>>>>
    > > >>>>>
    > > >>>>>
    > > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
    > > >>>>>>
    > > >>>>>> Hey All,
    > > >>>>>> I want to follow up on something that was briefly discussed at the
    > > >>>> summit
    > > >>>>>> this week.  Many people do not like the Changelog and feel like it
    > > can
    > > >>>> be a
    > > >>>>>> PITA to deal with.  One of the reasons we have the changelog is
    > > >>> because
    > > >>>>>> there are so many commits in a given release, that it is hard to
    > > comb
    > > >>>>>> through all of them to figure out what noteworthy changes or bug
    > > fixes
    > > >>>> went
    > > >>>>>> into the code.  One thing that may help with this problem is to use
    > > >>>> enable
    > > >>>>>> the squash and merge feature on Github for pull requests [1].   This
    > > >>>>>> feature would squash all commits in a PR into one commit.  If we
    > > pair
    > > >>>> the
    > > >>>>>> one commit with a good commit message, we would essentially get the
    > > >>>> ability
    > > >>>>>> to create the changelog just from the commits.
    > > >>>>>>
    > > >>>>>> So, I would like to propose that we enable the squash and merge
    > > >>> feature
    > > >>>> in
    > > >>>>>> the Traffic Control Github repo and use that going forward.
    > > Thoughts?
    > > >>>>>>
    > > >>>>>> Thanks,
    > > >>>>>> Dave
    > > >>>>>>
    > > >>>>>>
    > > >>>>>> [1] https://help.github.com/articles/about-pull-request-merges/
    > > >>>>>
    > > >>>>
    > > >>>>
    > > >>>
    > >
    

Re: [EXTERNAL] Re: Squash and Merge for PRs

["Fieck, Brennan" <Br...@comcast.com>]

I'm not gonna say +1 or -1, but I'd still like to chime in. I think it'd be nice to require PRs to squash things into atomic chunks. No commits that say "started x" or "minor fixes", but idk if it's reasonable to be able to boil every PR forever down to 70 characters. And sure you can make multiple PRs if changes are too big to fit in one commit message, but then it becomes really easy to become entwined in a chain of dependent PRs so that every time one of them gets merged you have to rebase them all.

Of course, a "reasonable squash" is pretty hard to define absolutely, and could actually just make PRs harder to review -  maybe it's not even enforceable at all.
________________________________________
From: Rawlin Peters <ra...@gmail.com>
Sent: Tuesday, October 23, 2018 11:14 AM
To: dev@trafficcontrol.apache.org
Subject: Re: [EXTERNAL] Re: Squash and Merge for PRs

+1 on enabling squash and merge.

This will not only make cherry picks easier to manage with respect to
merge conflicts, but it will also make the git log more useful because
we will be able to see the full changeset of the PR in a single commit
of the git log. Ideally, I'd also like the squashed commit to contain
a link to the actual github PR so that I can easily view comments, but
that is a proposal for another day.

- Rawlin

On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org> wrote:
>
> This seems to have stalled, so let me try to move it along.
>
> There is an ability to do "squash and merge" in github, we just need to
> agree that we want to enable it for our project.
>
> @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how to
> handle the multiple committers thing, but I assume we can squash into two
> commits and give one to each.  However, if we are doing something big
> enough to require that many commits from multiple users, it may be best as
> many PRs?  I don't think this is a very common situation, so I would like
> to avoid getting bogged down in what-ifs if possible.
>
> As for the Changelog, I am fine with enabling squash and merge and keeping
> the changelog as-is while we decide if squash and merge meets our needs.  I
> am also game to revisit different ways to do it, and I shouldn't have
> conflated that conversation with this one.  If someone wants to see that
> changed, we should start a new discussion.
>
> Leif brings up a good point about squash and merge making cherry picks
> easier so I think we should consider it for that point alone.
>
> So, what I am really asking is:  "do we think it is a good idea to enable
> squash and merge on our Github repo so that we can reduce the amount of
> commits per PR?"  I would like to get consensus for or against.  If it is
> not clear already, I am for.
>
>
> Thanks,
> Dave
>
>
> On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <De...@comcast.com>
> wrote:
>
> > I'll be honest, I think the simplest thing is still to just require a
> > changelog message, added to a central file, to be included in the commit.
> > We've been going over and over this for months instead of actually writing
> > change logs.  Let's just propose it and put it to a vote.
> >
> > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <ra...@gmail.com>
> > wrote:
> > >
> > > I'm +1 on squashing and merging, but I think we need a combination of
> > > a number of things to really have a decent changelog. Even if we have
> > > awesome commit messages and squashed PRs, there's still way too many
> > > PRs in a release to just generate a decent changelog from the PR
> > > titles and/or commit messages. We still need a way to logically
> > > separate PRs into bugfixes, new features, breaking changes, etc. We
> > > could get some of that separation using github tags, but what if there
> > > are multiple PRs that compose one new high-level feature? At that
> > > point I think we'd have to create a new tag for every new high-level
> > > feature being worked on in order to group those PRs under the same
> > > heading. There's also the problem that only committers can tag issues,
> > > so it adds to the burden of committers to make sure all non-committer
> > > issues/PRs are properly tagged.
> > >
> > > The main headache with doing a manually-curated changelog is the merge
> > > conflicts it causes, but I think we can easily avoid that problem by
> > > using a similar approach to reno [1] which breaks individual notes out
> > > into their own separate yaml files which are then parsed and merged
> > > into a single, cohesive changelog for each release. What that means is
> > > basically every logical bugfix/new feature/upgrade note gets its own
> > > file which avoids merge conflicts caused by just adding a line to the
> > > end of a CHANGELOG.md file every time.
> > >
> > > - Rawlin
> > >
> > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com>
> > wrote:
> > >>
> > >> is there a setting in github to enable "squash and merge" vs "rebase and
> > >> merge"?
> > >>
> > >> i do have a couple of concerns however:
> > >>
> > >> 1. what happens when 2+ people contribute to a PR? This is a pretty
> > common
> > >> scenario. If Bob and Sam both work on PR#1 is all the commit history for
> > >> Sam lost?
> > >> 2. i can see benefits of having granular history...to a point...
> > >>
> > >> The more I think about this. Why is squash needed? Commits are grouped
> > by
> > >> PRs. Why isn't our change log just a list of PR merged since the last
> > >> release? If a PR represents a working unit (as it should), PR titles
> > should
> > >> be sufficient, no?
> > >>
> > >> For example, this is what our internal release change log looks like.
> > >>
> > >> Changelog from 2526569a to 284555703
> > >> ========================================
> > >>
> > >> These changes are deployed in branch deploy-20181002.
> > >>
> > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
> > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> > cachegroups/dses
> > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
> > >> (Robert Butts, Sep 13 2018)
> > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
> > >> steeringtargets (Robert Butts, Sep 11 2018)
> > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
> > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > >> PR 2806: Add sub to validate user input when running postinstall script
> > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate
> > user
> > >> input when running postinstall script changes default cdn name to not
> > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall
> > cdn
> > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
> > >>        70d8893335b63db006974932341378717c42701c: Changing back the
> > default
> > >> cdn name value (Dylan Souza, Sep 21 2018)
> > >> PR 2812: remove traffic_monitor_java
> > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> > traffic_monitor_java
> > >> (Dan Kirkwood, Sep 12 2018)
> > >>
> > >>
> > >> [truncated]
> > >>
> > >>
> > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
> > >>>
> > >>> Well, our "curated" changelogs are missing A LOT of information on what
> > >>> actually went into the release, which therefore makes it not useful.
> > If we
> > >>> were better about commit messages and used squash and merge, then we
> > would
> > >>> at least have something for every PR that was merged.
> > >>>
> > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
> > >>>
> > >>>>> I think this just shifts the burden from writing a changelog entry to
> > >>>> writing a good commit entry.
> > >>>>
> > >>>>
> > >>>> I agree, and I think that’s where that burden belongs. (And I _really_
> > >>>> hate merge conflicts).
> > >>>>
> > >>>>> There might be fewer commit entries if we squash and merge, but I’m
> > >>>> doubtful that it would be as valuable as our “curated” changelogs.
> > >>>>
> > >>>> Both are dependent on how disciplined we are.
> > >>>>
> > >>>> I’m +1 on Squash and Merge.
> > >>>>
> > >>>> Cheers,
> > >>>> JvD
> > >>>>
> > >>>>
> > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > >>>> <ef...@cisco.com.INVALID> wrote:
> > >>>>>
> > >>>>> I think this just shifts the burden from writing a changelog entry to
> > >>>> writing a good commit entry.
> > >>>>>
> > >>>>> There might be fewer commit entries if we squash and merge, but I’m
> > >>>> doubtful that it would be as valuable as our “curated” changelogs.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> > >>>>>>
> > >>>>>> Hey All,
> > >>>>>> I want to follow up on something that was briefly discussed at the
> > >>>> summit
> > >>>>>> this week.  Many people do not like the Changelog and feel like it
> > can
> > >>>> be a
> > >>>>>> PITA to deal with.  One of the reasons we have the changelog is
> > >>> because
> > >>>>>> there are so many commits in a given release, that it is hard to
> > comb
> > >>>>>> through all of them to figure out what noteworthy changes or bug
> > fixes
> > >>>> went
> > >>>>>> into the code.  One thing that may help with this problem is to use
> > >>>> enable
> > >>>>>> the squash and merge feature on Github for pull requests [1].   This
> > >>>>>> feature would squash all commits in a PR into one commit.  If we
> > pair
> > >>>> the
> > >>>>>> one commit with a good commit message, we would essentially get the
> > >>>> ability
> > >>>>>> to create the changelog just from the commits.
> > >>>>>>
> > >>>>>> So, I would like to propose that we enable the squash and merge
> > >>> feature
> > >>>> in
> > >>>>>> the Traffic Control Github repo and use that going forward.
> > Thoughts?
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Dave
> > >>>>>>
> > >>>>>>
> > >>>>>> [1] https://help.github.com/articles/about-pull-request-merges/
> > >>>>>
> > >>>>
> > >>>>
> > >>>
> >

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Rawlin Peters <ra...@gmail.com>]

+1 on enabling squash and merge.

This will not only make cherry picks easier to manage with respect to
merge conflicts, but it will also make the git log more useful because
we will be able to see the full changeset of the PR in a single commit
of the git log. Ideally, I'd also like the squashed commit to contain
a link to the actual github PR so that I can easily view comments, but
that is a proposal for another day.

- Rawlin

On Tue, Oct 23, 2018 at 10:41 AM Dave Neuman <ne...@apache.org> wrote:
>
> This seems to have stalled, so let me try to move it along.
>
> There is an ability to do "squash and merge" in github, we just need to
> agree that we want to enable it for our project.
>
> @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how to
> handle the multiple committers thing, but I assume we can squash into two
> commits and give one to each.  However, if we are doing something big
> enough to require that many commits from multiple users, it may be best as
> many PRs?  I don't think this is a very common situation, so I would like
> to avoid getting bogged down in what-ifs if possible.
>
> As for the Changelog, I am fine with enabling squash and merge and keeping
> the changelog as-is while we decide if squash and merge meets our needs.  I
> am also game to revisit different ways to do it, and I shouldn't have
> conflated that conversation with this one.  If someone wants to see that
> changed, we should start a new discussion.
>
> Leif brings up a good point about squash and merge making cherry picks
> easier so I think we should consider it for that point alone.
>
> So, what I am really asking is:  "do we think it is a good idea to enable
> squash and merge on our Github repo so that we can reduce the amount of
> commits per PR?"  I would like to get consensus for or against.  If it is
> not clear already, I am for.
>
>
> Thanks,
> Dave
>
>
> On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <De...@comcast.com>
> wrote:
>
> > I'll be honest, I think the simplest thing is still to just require a
> > changelog message, added to a central file, to be included in the commit.
> > We've been going over and over this for months instead of actually writing
> > change logs.  Let's just propose it and put it to a vote.
> >
> > > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <ra...@gmail.com>
> > wrote:
> > >
> > > I'm +1 on squashing and merging, but I think we need a combination of
> > > a number of things to really have a decent changelog. Even if we have
> > > awesome commit messages and squashed PRs, there's still way too many
> > > PRs in a release to just generate a decent changelog from the PR
> > > titles and/or commit messages. We still need a way to logically
> > > separate PRs into bugfixes, new features, breaking changes, etc. We
> > > could get some of that separation using github tags, but what if there
> > > are multiple PRs that compose one new high-level feature? At that
> > > point I think we'd have to create a new tag for every new high-level
> > > feature being worked on in order to group those PRs under the same
> > > heading. There's also the problem that only committers can tag issues,
> > > so it adds to the burden of committers to make sure all non-committer
> > > issues/PRs are properly tagged.
> > >
> > > The main headache with doing a manually-curated changelog is the merge
> > > conflicts it causes, but I think we can easily avoid that problem by
> > > using a similar approach to reno [1] which breaks individual notes out
> > > into their own separate yaml files which are then parsed and merged
> > > into a single, cohesive changelog for each release. What that means is
> > > basically every logical bugfix/new feature/upgrade note gets its own
> > > file which avoids merge conflicts caused by just adding a line to the
> > > end of a CHANGELOG.md file every time.
> > >
> > > - Rawlin
> > >
> > > [1] https://docs.openstack.org/reno/latest/user/usage.html
> > >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com>
> > wrote:
> > >>
> > >> is there a setting in github to enable "squash and merge" vs "rebase and
> > >> merge"?
> > >>
> > >> i do have a couple of concerns however:
> > >>
> > >> 1. what happens when 2+ people contribute to a PR? This is a pretty
> > common
> > >> scenario. If Bob and Sam both work on PR#1 is all the commit history for
> > >> Sam lost?
> > >> 2. i can see benefits of having granular history...to a point...
> > >>
> > >> The more I think about this. Why is squash needed? Commits are grouped
> > by
> > >> PRs. Why isn't our change log just a list of PR merged since the last
> > >> release? If a PR represents a working unit (as it should), PR titles
> > should
> > >> be sufficient, no?
> > >>
> > >> For example, this is what our internal release change log looks like.
> > >>
> > >> Changelog from 2526569a to 284555703
> > >> ========================================
> > >>
> > >> These changes are deployed in branch deploy-20181002.
> > >>
> > >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> > >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> > >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
> > >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> > cachegroups/dses
> > >> API test, client funcs (Robert Butts, Jun 28 2018)
> > >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
> > >> (Robert Butts, Sep 13 2018)
> > >> PR 2803: Add Traffic Ops Golang Steering Targets
> > >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> > >> steering/id/targets (Robert Butts, Jul 10 2018)
> > >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
> > >> steeringtargets (Robert Butts, Sep 11 2018)
> > >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
> > >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> > >> PR 2806: Add sub to validate user input when running postinstall script
> > >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate
> > user
> > >> input when running postinstall script changes default cdn name to not
> > >> include an underscore. (Dylan Souza, Sep 11 2018)
> > >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall
> > cdn
> > >> name regex to include underscores (Dylan Souza, Sep 21 2018)
> > >>        70d8893335b63db006974932341378717c42701c: Changing back the
> > default
> > >> cdn name value (Dylan Souza, Sep 21 2018)
> > >> PR 2812: remove traffic_monitor_java
> > >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> > traffic_monitor_java
> > >> (Dan Kirkwood, Sep 12 2018)
> > >>
> > >>
> > >> [truncated]
> > >>
> > >>
> > >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
> > >>>
> > >>> Well, our "curated" changelogs are missing A LOT of information on what
> > >>> actually went into the release, which therefore makes it not useful.
> > If we
> > >>> were better about commit messages and used squash and merge, then we
> > would
> > >>> at least have something for every PR that was merged.
> > >>>
> > >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
> > >>>
> > >>>>> I think this just shifts the burden from writing a changelog entry to
> > >>>> writing a good commit entry.
> > >>>>
> > >>>>
> > >>>> I agree, and I think that’s where that burden belongs. (And I _really_
> > >>>> hate merge conflicts).
> > >>>>
> > >>>>> There might be fewer commit entries if we squash and merge, but I’m
> > >>>> doubtful that it would be as valuable as our “curated” changelogs.
> > >>>>
> > >>>> Both are dependent on how disciplined we are.
> > >>>>
> > >>>> I’m +1 on Squash and Merge.
> > >>>>
> > >>>> Cheers,
> > >>>> JvD
> > >>>>
> > >>>>
> > >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > >>>> <ef...@cisco.com.INVALID> wrote:
> > >>>>>
> > >>>>> I think this just shifts the burden from writing a changelog entry to
> > >>>> writing a good commit entry.
> > >>>>>
> > >>>>> There might be fewer commit entries if we squash and merge, but I’m
> > >>>> doubtful that it would be as valuable as our “curated” changelogs.
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> > >>>>>>
> > >>>>>> Hey All,
> > >>>>>> I want to follow up on something that was briefly discussed at the
> > >>>> summit
> > >>>>>> this week.  Many people do not like the Changelog and feel like it
> > can
> > >>>> be a
> > >>>>>> PITA to deal with.  One of the reasons we have the changelog is
> > >>> because
> > >>>>>> there are so many commits in a given release, that it is hard to
> > comb
> > >>>>>> through all of them to figure out what noteworthy changes or bug
> > fixes
> > >>>> went
> > >>>>>> into the code.  One thing that may help with this problem is to use
> > >>>> enable
> > >>>>>> the squash and merge feature on Github for pull requests [1].   This
> > >>>>>> feature would squash all commits in a PR into one commit.  If we
> > pair
> > >>>> the
> > >>>>>> one commit with a good commit message, we would essentially get the
> > >>>> ability
> > >>>>>> to create the changelog just from the commits.
> > >>>>>>
> > >>>>>> So, I would like to propose that we enable the squash and merge
> > >>> feature
> > >>>> in
> > >>>>>> the Traffic Control Github repo and use that going forward.
> > Thoughts?
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Dave
> > >>>>>>
> > >>>>>>
> > >>>>>> [1] https://help.github.com/articles/about-pull-request-merges/
> > >>>>>
> > >>>>
> > >>>>
> > >>>
> >

Re: [EXTERNAL] Re: Squash and Merge for PRs

["Gelinas, Derek" <De...@comcast.com>]

+1 on the squishing.

> On Oct 23, 2018, at 12:41 PM, Dave Neuman <ne...@apache.org> wrote:
> 
> This seems to have stalled, so let me try to move it along.
> 
> There is an ability to do "squash and merge" in github, we just need to
> agree that we want to enable it for our project.
> 
> @Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how to
> handle the multiple committers thing, but I assume we can squash into two
> commits and give one to each.  However, if we are doing something big
> enough to require that many commits from multiple users, it may be best as
> many PRs?  I don't think this is a very common situation, so I would like
> to avoid getting bogged down in what-ifs if possible.
> 
> As for the Changelog, I am fine with enabling squash and merge and keeping
> the changelog as-is while we decide if squash and merge meets our needs.  I
> am also game to revisit different ways to do it, and I shouldn't have
> conflated that conversation with this one.  If someone wants to see that
> changed, we should start a new discussion.
> 
> Leif brings up a good point about squash and merge making cherry picks
> easier so I think we should consider it for that point alone.
> 
> So, what I am really asking is:  "do we think it is a good idea to enable
> squash and merge on our Github repo so that we can reduce the amount of
> commits per PR?"  I would like to get consensus for or against.  If it is
> not clear already, I am for.
> 
> 
> Thanks,
> Dave
> 
> 
> On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <De...@comcast.com>
> wrote:
> 
>> I'll be honest, I think the simplest thing is still to just require a
>> changelog message, added to a central file, to be included in the commit.
>> We've been going over and over this for months instead of actually writing
>> change logs.  Let's just propose it and put it to a vote.
>> 
>>> On Oct 18, 2018, at 4:33 PM, Rawlin Peters <ra...@gmail.com>
>> wrote:
>>> 
>>> I'm +1 on squashing and merging, but I think we need a combination of
>>> a number of things to really have a decent changelog. Even if we have
>>> awesome commit messages and squashed PRs, there's still way too many
>>> PRs in a release to just generate a decent changelog from the PR
>>> titles and/or commit messages. We still need a way to logically
>>> separate PRs into bugfixes, new features, breaking changes, etc. We
>>> could get some of that separation using github tags, but what if there
>>> are multiple PRs that compose one new high-level feature? At that
>>> point I think we'd have to create a new tag for every new high-level
>>> feature being worked on in order to group those PRs under the same
>>> heading. There's also the problem that only committers can tag issues,
>>> so it adds to the burden of committers to make sure all non-committer
>>> issues/PRs are properly tagged.
>>> 
>>> The main headache with doing a manually-curated changelog is the merge
>>> conflicts it causes, but I think we can easily avoid that problem by
>>> using a similar approach to reno [1] which breaks individual notes out
>>> into their own separate yaml files which are then parsed and merged
>>> into a single, cohesive changelog for each release. What that means is
>>> basically every logical bugfix/new feature/upgrade note gets its own
>>> file which avoids merge conflicts caused by just adding a line to the
>>> end of a CHANGELOG.md file every time.
>>> 
>>> - Rawlin
>>> 
>>> [1] https://docs.openstack.org/reno/latest/user/usage.html
>>>> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com>
>> wrote:
>>>> 
>>>> is there a setting in github to enable "squash and merge" vs "rebase and
>>>> merge"?
>>>> 
>>>> i do have a couple of concerns however:
>>>> 
>>>> 1. what happens when 2+ people contribute to a PR? This is a pretty
>> common
>>>> scenario. If Bob and Sam both work on PR#1 is all the commit history for
>>>> Sam lost?
>>>> 2. i can see benefits of having granular history...to a point...
>>>> 
>>>> The more I think about this. Why is squash needed? Commits are grouped
>> by
>>>> PRs. Why isn't our change log just a list of PR merged since the last
>>>> release? If a PR represents a working unit (as it should), PR titles
>> should
>>>> be sufficient, no?
>>>> 
>>>> For example, this is what our internal release change log looks like.
>>>> 
>>>> Changelog from 2526569a to 284555703
>>>> ========================================
>>>> 
>>>> These changes are deployed in branch deploy-20181002.
>>>> 
>>>> PR 2300: Add TO Go cachegroups/id/deliveryservices
>>>>       65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
>>>> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
>>>>       130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
>> cachegroups/dses
>>>> API test, client funcs (Robert Butts, Jun 28 2018)
>>>>       5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
>>>> (Robert Butts, Sep 13 2018)
>>>> PR 2803: Add Traffic Ops Golang Steering Targets
>>>>       8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
>>>> steering/id/targets (Robert Butts, Jul 10 2018)
>>>>       5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
>>>> steeringtargets (Robert Butts, Sep 11 2018)
>>>>       eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
>>>> steeringtarget funcs (Robert Butts, Sep 12 2018)
>>>> PR 2806: Add sub to validate user input when running postinstall script
>>>>       e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate
>> user
>>>> input when running postinstall script changes default cdn name to not
>>>> include an underscore. (Dylan Souza, Sep 11 2018)
>>>>       9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall
>> cdn
>>>> name regex to include underscores (Dylan Souza, Sep 21 2018)
>>>>       70d8893335b63db006974932341378717c42701c: Changing back the
>> default
>>>> cdn name value (Dylan Souza, Sep 21 2018)
>>>> PR 2812: remove traffic_monitor_java
>>>>       4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
>> traffic_monitor_java
>>>> (Dan Kirkwood, Sep 12 2018)
>>>> 
>>>> 
>>>> [truncated]
>>>> 
>>>> 
>>>>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
>>>>> 
>>>>> Well, our "curated" changelogs are missing A LOT of information on what
>>>>> actually went into the release, which therefore makes it not useful.
>> If we
>>>>> were better about commit messages and used squash and merge, then we
>> would
>>>>> at least have something for every PR that was merged.
>>>>> 
>>>>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
>>>>> 
>>>>>>> I think this just shifts the burden from writing a changelog entry to
>>>>>> writing a good commit entry.
>>>>>> 
>>>>>> 
>>>>>> I agree, and I think that’s where that burden belongs. (And I _really_
>>>>>> hate merge conflicts).
>>>>>> 
>>>>>>> There might be fewer commit entries if we squash and merge, but I’m
>>>>>> doubtful that it would be as valuable as our “curated” changelogs.
>>>>>> 
>>>>>> Both are dependent on how disciplined we are.
>>>>>> 
>>>>>> I’m +1 on Squash and Merge.
>>>>>> 
>>>>>> Cheers,
>>>>>> JvD
>>>>>> 
>>>>>> 
>>>>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
>>>>>> <ef...@cisco.com.INVALID> wrote:
>>>>>>> 
>>>>>>> I think this just shifts the burden from writing a changelog entry to
>>>>>> writing a good commit entry.
>>>>>>> 
>>>>>>> There might be fewer commit entries if we squash and merge, but I’m
>>>>>> doubtful that it would be as valuable as our “curated” changelogs.
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
>>>>>>>> 
>>>>>>>> Hey All,
>>>>>>>> I want to follow up on something that was briefly discussed at the
>>>>>> summit
>>>>>>>> this week.  Many people do not like the Changelog and feel like it
>> can
>>>>>> be a
>>>>>>>> PITA to deal with.  One of the reasons we have the changelog is
>>>>> because
>>>>>>>> there are so many commits in a given release, that it is hard to
>> comb
>>>>>>>> through all of them to figure out what noteworthy changes or bug
>> fixes
>>>>>> went
>>>>>>>> into the code.  One thing that may help with this problem is to use
>>>>>> enable
>>>>>>>> the squash and merge feature on Github for pull requests [1].   This
>>>>>>>> feature would squash all commits in a PR into one commit.  If we
>> pair
>>>>>> the
>>>>>>>> one commit with a good commit message, we would essentially get the
>>>>>> ability
>>>>>>>> to create the changelog just from the commits.
>>>>>>>> 
>>>>>>>> So, I would like to propose that we enable the squash and merge
>>>>> feature
>>>>>> in
>>>>>>>> the Traffic Control Github repo and use that going forward.
>> Thoughts?
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Dave
>>>>>>>> 
>>>>>>>> 
>>>>>>>> [1] https://help.github.com/articles/about-pull-request-merges/
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>> 

Re: [EXTERNAL] Re: Squash and Merge for PRs

[Dave Neuman <ne...@apache.org>]

This seems to have stalled, so let me try to move it along.

There is an ability to do "squash and merge" in github, we just need to
agree that we want to enable it for our project.

@Jeremy Mitchell <mi...@gmail.com> I am not sure exactly how to
handle the multiple committers thing, but I assume we can squash into two
commits and give one to each.  However, if we are doing something big
enough to require that many commits from multiple users, it may be best as
many PRs?  I don't think this is a very common situation, so I would like
to avoid getting bogged down in what-ifs if possible.

As for the Changelog, I am fine with enabling squash and merge and keeping
the changelog as-is while we decide if squash and merge meets our needs.  I
am also game to revisit different ways to do it, and I shouldn't have
conflated that conversation with this one.  If someone wants to see that
changed, we should start a new discussion.

Leif brings up a good point about squash and merge making cherry picks
easier so I think we should consider it for that point alone.

So, what I am really asking is:  "do we think it is a good idea to enable
squash and merge on our Github repo so that we can reduce the amount of
commits per PR?"  I would like to get consensus for or against.  If it is
not clear already, I am for.


Thanks,
Dave


On Thu, Oct 18, 2018 at 2:41 PM Gelinas, Derek <De...@comcast.com>
wrote:

> I'll be honest, I think the simplest thing is still to just require a
> changelog message, added to a central file, to be included in the commit.
> We've been going over and over this for months instead of actually writing
> change logs.  Let's just propose it and put it to a vote.
>
> > On Oct 18, 2018, at 4:33 PM, Rawlin Peters <ra...@gmail.com>
> wrote:
> >
> > I'm +1 on squashing and merging, but I think we need a combination of
> > a number of things to really have a decent changelog. Even if we have
> > awesome commit messages and squashed PRs, there's still way too many
> > PRs in a release to just generate a decent changelog from the PR
> > titles and/or commit messages. We still need a way to logically
> > separate PRs into bugfixes, new features, breaking changes, etc. We
> > could get some of that separation using github tags, but what if there
> > are multiple PRs that compose one new high-level feature? At that
> > point I think we'd have to create a new tag for every new high-level
> > feature being worked on in order to group those PRs under the same
> > heading. There's also the problem that only committers can tag issues,
> > so it adds to the burden of committers to make sure all non-committer
> > issues/PRs are properly tagged.
> >
> > The main headache with doing a manually-curated changelog is the merge
> > conflicts it causes, but I think we can easily avoid that problem by
> > using a similar approach to reno [1] which breaks individual notes out
> > into their own separate yaml files which are then parsed and merged
> > into a single, cohesive changelog for each release. What that means is
> > basically every logical bugfix/new feature/upgrade note gets its own
> > file which avoids merge conflicts caused by just adding a line to the
> > end of a CHANGELOG.md file every time.
> >
> > - Rawlin
> >
> > [1] https://docs.openstack.org/reno/latest/user/usage.html
> >> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com>
> wrote:
> >>
> >> is there a setting in github to enable "squash and merge" vs "rebase and
> >> merge"?
> >>
> >> i do have a couple of concerns however:
> >>
> >> 1. what happens when 2+ people contribute to a PR? This is a pretty
> common
> >> scenario. If Bob and Sam both work on PR#1 is all the commit history for
> >> Sam lost?
> >> 2. i can see benefits of having granular history...to a point...
> >>
> >> The more I think about this. Why is squash needed? Commits are grouped
> by
> >> PRs. Why isn't our change log just a list of PR merged since the last
> >> release? If a PR represents a working unit (as it should), PR titles
> should
> >> be sufficient, no?
> >>
> >> For example, this is what our internal release change log looks like.
> >>
> >> Changelog from 2526569a to 284555703
> >> ========================================
> >>
> >> These changes are deployed in branch deploy-20181002.
> >>
> >> PR 2300: Add TO Go cachegroups/id/deliveryservices
> >>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> >> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
> >>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go
> cachegroups/dses
> >> API test, client funcs (Robert Butts, Jun 28 2018)
> >>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
> >> (Robert Butts, Sep 13 2018)
> >> PR 2803: Add Traffic Ops Golang Steering Targets
> >>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> >> steering/id/targets (Robert Butts, Jul 10 2018)
> >>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
> >> steeringtargets (Robert Butts, Sep 11 2018)
> >>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
> >> steeringtarget funcs (Robert Butts, Sep 12 2018)
> >> PR 2806: Add sub to validate user input when running postinstall script
> >>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate
> user
> >> input when running postinstall script changes default cdn name to not
> >> include an underscore. (Dylan Souza, Sep 11 2018)
> >>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall
> cdn
> >> name regex to include underscores (Dylan Souza, Sep 21 2018)
> >>        70d8893335b63db006974932341378717c42701c: Changing back the
> default
> >> cdn name value (Dylan Souza, Sep 21 2018)
> >> PR 2812: remove traffic_monitor_java
> >>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove
> traffic_monitor_java
> >> (Dan Kirkwood, Sep 12 2018)
> >>
> >>
> >> [truncated]
> >>
> >>
> >>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
> >>>
> >>> Well, our "curated" changelogs are missing A LOT of information on what
> >>> actually went into the release, which therefore makes it not useful.
> If we
> >>> were better about commit messages and used squash and merge, then we
> would
> >>> at least have something for every PR that was merged.
> >>>
> >>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
> >>>
> >>>>> I think this just shifts the burden from writing a changelog entry to
> >>>> writing a good commit entry.
> >>>>
> >>>>
> >>>> I agree, and I think that’s where that burden belongs. (And I _really_
> >>>> hate merge conflicts).
> >>>>
> >>>>> There might be fewer commit entries if we squash and merge, but I’m
> >>>> doubtful that it would be as valuable as our “curated” changelogs.
> >>>>
> >>>> Both are dependent on how disciplined we are.
> >>>>
> >>>> I’m +1 on Squash and Merge.
> >>>>
> >>>> Cheers,
> >>>> JvD
> >>>>
> >>>>
> >>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> >>>> <ef...@cisco.com.INVALID> wrote:
> >>>>>
> >>>>> I think this just shifts the burden from writing a changelog entry to
> >>>> writing a good commit entry.
> >>>>>
> >>>>> There might be fewer commit entries if we squash and merge, but I’m
> >>>> doubtful that it would be as valuable as our “curated” changelogs.
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> >>>>>>
> >>>>>> Hey All,
> >>>>>> I want to follow up on something that was briefly discussed at the
> >>>> summit
> >>>>>> this week.  Many people do not like the Changelog and feel like it
> can
> >>>> be a
> >>>>>> PITA to deal with.  One of the reasons we have the changelog is
> >>> because
> >>>>>> there are so many commits in a given release, that it is hard to
> comb
> >>>>>> through all of them to figure out what noteworthy changes or bug
> fixes
> >>>> went
> >>>>>> into the code.  One thing that may help with this problem is to use
> >>>> enable
> >>>>>> the squash and merge feature on Github for pull requests [1].   This
> >>>>>> feature would squash all commits in a PR into one commit.  If we
> pair
> >>>> the
> >>>>>> one commit with a good commit message, we would essentially get the
> >>>> ability
> >>>>>> to create the changelog just from the commits.
> >>>>>>
> >>>>>> So, I would like to propose that we enable the squash and merge
> >>> feature
> >>>> in
> >>>>>> the Traffic Control Github repo and use that going forward.
> Thoughts?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Dave
> >>>>>>
> >>>>>>
> >>>>>> [1] https://help.github.com/articles/about-pull-request-merges/
> >>>>>
> >>>>
> >>>>
> >>>
>

Re: [EXTERNAL] Re: Squash and Merge for PRs

["Gelinas, Derek" <De...@comcast.com>]

I'll be honest, I think the simplest thing is still to just require a changelog message, added to a central file, to be included in the commit. We've been going over and over this for months instead of actually writing change logs.  Let's just propose it and put it to a vote.

> On Oct 18, 2018, at 4:33 PM, Rawlin Peters <ra...@gmail.com> wrote:
> 
> I'm +1 on squashing and merging, but I think we need a combination of
> a number of things to really have a decent changelog. Even if we have
> awesome commit messages and squashed PRs, there's still way too many
> PRs in a release to just generate a decent changelog from the PR
> titles and/or commit messages. We still need a way to logically
> separate PRs into bugfixes, new features, breaking changes, etc. We
> could get some of that separation using github tags, but what if there
> are multiple PRs that compose one new high-level feature? At that
> point I think we'd have to create a new tag for every new high-level
> feature being worked on in order to group those PRs under the same
> heading. There's also the problem that only committers can tag issues,
> so it adds to the burden of committers to make sure all non-committer
> issues/PRs are properly tagged.
> 
> The main headache with doing a manually-curated changelog is the merge
> conflicts it causes, but I think we can easily avoid that problem by
> using a similar approach to reno [1] which breaks individual notes out
> into their own separate yaml files which are then parsed and merged
> into a single, cohesive changelog for each release. What that means is
> basically every logical bugfix/new feature/upgrade note gets its own
> file which avoids merge conflicts caused by just adding a line to the
> end of a CHANGELOG.md file every time.
> 
> - Rawlin
> 
> [1] https://docs.openstack.org/reno/latest/user/usage.html
>> On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com> wrote:
>> 
>> is there a setting in github to enable "squash and merge" vs "rebase and
>> merge"?
>> 
>> i do have a couple of concerns however:
>> 
>> 1. what happens when 2+ people contribute to a PR? This is a pretty common
>> scenario. If Bob and Sam both work on PR#1 is all the commit history for
>> Sam lost?
>> 2. i can see benefits of having granular history...to a point...
>> 
>> The more I think about this. Why is squash needed? Commits are grouped by
>> PRs. Why isn't our change log just a list of PR merged since the last
>> release? If a PR represents a working unit (as it should), PR titles should
>> be sufficient, no?
>> 
>> For example, this is what our internal release change log looks like.
>> 
>> Changelog from 2526569a to 284555703
>> ========================================
>> 
>> These changes are deployed in branch deploy-20181002.
>> 
>> PR 2300: Add TO Go cachegroups/id/deliveryservices
>>        65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
>> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
>>        130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go cachegroups/dses
>> API test, client funcs (Robert Butts, Jun 28 2018)
>>        5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
>> (Robert Butts, Sep 13 2018)
>> PR 2803: Add Traffic Ops Golang Steering Targets
>>        8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
>> steering/id/targets (Robert Butts, Jul 10 2018)
>>        5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
>> steeringtargets (Robert Butts, Sep 11 2018)
>>        eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
>> steeringtarget funcs (Robert Butts, Sep 12 2018)
>> PR 2806: Add sub to validate user input when running postinstall script
>>        e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate user
>> input when running postinstall script changes default cdn name to not
>> include an underscore. (Dylan Souza, Sep 11 2018)
>>        9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall cdn
>> name regex to include underscores (Dylan Souza, Sep 21 2018)
>>        70d8893335b63db006974932341378717c42701c: Changing back the default
>> cdn name value (Dylan Souza, Sep 21 2018)
>> PR 2812: remove traffic_monitor_java
>>        4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove traffic_monitor_java
>> (Dan Kirkwood, Sep 12 2018)
>> 
>> 
>> [truncated]
>> 
>> 
>>> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
>>> 
>>> Well, our "curated" changelogs are missing A LOT of information on what
>>> actually went into the release, which therefore makes it not useful.  If we
>>> were better about commit messages and used squash and merge, then we would
>>> at least have something for every PR that was merged.
>>> 
>>> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
>>> 
>>>>> I think this just shifts the burden from writing a changelog entry to
>>>> writing a good commit entry.
>>>> 
>>>> 
>>>> I agree, and I think that’s where that burden belongs. (And I _really_
>>>> hate merge conflicts).
>>>> 
>>>>> There might be fewer commit entries if we squash and merge, but I’m
>>>> doubtful that it would be as valuable as our “curated” changelogs.
>>>> 
>>>> Both are dependent on how disciplined we are.
>>>> 
>>>> I’m +1 on Squash and Merge.
>>>> 
>>>> Cheers,
>>>> JvD
>>>> 
>>>> 
>>>>> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
>>>> <ef...@cisco.com.INVALID> wrote:
>>>>> 
>>>>> I think this just shifts the burden from writing a changelog entry to
>>>> writing a good commit entry.
>>>>> 
>>>>> There might be fewer commit entries if we squash and merge, but I’m
>>>> doubtful that it would be as valuable as our “curated” changelogs.
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
>>>>>> 
>>>>>> Hey All,
>>>>>> I want to follow up on something that was briefly discussed at the
>>>> summit
>>>>>> this week.  Many people do not like the Changelog and feel like it can
>>>> be a
>>>>>> PITA to deal with.  One of the reasons we have the changelog is
>>> because
>>>>>> there are so many commits in a given release, that it is hard to comb
>>>>>> through all of them to figure out what noteworthy changes or bug fixes
>>>> went
>>>>>> into the code.  One thing that may help with this problem is to use
>>>> enable
>>>>>> the squash and merge feature on Github for pull requests [1].   This
>>>>>> feature would squash all commits in a PR into one commit.  If we pair
>>>> the
>>>>>> one commit with a good commit message, we would essentially get the
>>>> ability
>>>>>> to create the changelog just from the commits.
>>>>>> 
>>>>>> So, I would like to propose that we enable the squash and merge
>>> feature
>>>> in
>>>>>> the Traffic Control Github repo and use that going forward.  Thoughts?
>>>>>> 
>>>>>> Thanks,
>>>>>> Dave
>>>>>> 
>>>>>> 
>>>>>> [1] https://help.github.com/articles/about-pull-request-merges/
>>>>> 
>>>> 
>>>> 
>>> 

Re: Squash and Merge for PRs

[Rawlin Peters <ra...@gmail.com>]

I'm +1 on squashing and merging, but I think we need a combination of
a number of things to really have a decent changelog. Even if we have
awesome commit messages and squashed PRs, there's still way too many
PRs in a release to just generate a decent changelog from the PR
titles and/or commit messages. We still need a way to logically
separate PRs into bugfixes, new features, breaking changes, etc. We
could get some of that separation using github tags, but what if there
are multiple PRs that compose one new high-level feature? At that
point I think we'd have to create a new tag for every new high-level
feature being worked on in order to group those PRs under the same
heading. There's also the problem that only committers can tag issues,
so it adds to the burden of committers to make sure all non-committer
issues/PRs are properly tagged.

The main headache with doing a manually-curated changelog is the merge
conflicts it causes, but I think we can easily avoid that problem by
using a similar approach to reno [1] which breaks individual notes out
into their own separate yaml files which are then parsed and merged
into a single, cohesive changelog for each release. What that means is
basically every logical bugfix/new feature/upgrade note gets its own
file which avoids merge conflicts caused by just adding a line to the
end of a CHANGELOG.md file every time.

- Rawlin

[1] https://docs.openstack.org/reno/latest/user/usage.html
On Thu, Oct 18, 2018 at 1:45 PM Jeremy Mitchell <mi...@gmail.com> wrote:
>
> is there a setting in github to enable "squash and merge" vs "rebase and
> merge"?
>
> i do have a couple of concerns however:
>
> 1. what happens when 2+ people contribute to a PR? This is a pretty common
> scenario. If Bob and Sam both work on PR#1 is all the commit history for
> Sam lost?
> 2. i can see benefits of having granular history...to a point...
>
> The more I think about this. Why is squash needed? Commits are grouped by
> PRs. Why isn't our change log just a list of PR merged since the last
> release? If a PR represents a working unit (as it should), PR titles should
> be sufficient, no?
>
>  For example, this is what our internal release change log looks like.
>
> Changelog from 2526569a to 284555703
> ========================================
>
> These changes are deployed in branch deploy-20181002.
>
> PR 2300: Add TO Go cachegroups/id/deliveryservices
>         65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
> cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
>         130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go cachegroups/dses
> API test, client funcs (Robert Butts, Jun 28 2018)
>         5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
> (Robert Butts, Sep 13 2018)
> PR 2803: Add Traffic Ops Golang Steering Targets
>         8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
> steering/id/targets (Robert Butts, Jul 10 2018)
>         5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
> steeringtargets (Robert Butts, Sep 11 2018)
>         eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
> steeringtarget funcs (Robert Butts, Sep 12 2018)
> PR 2806: Add sub to validate user input when running postinstall script
>         e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate user
> input when running postinstall script changes default cdn name to not
> include an underscore. (Dylan Souza, Sep 11 2018)
>         9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall cdn
> name regex to include underscores (Dylan Souza, Sep 21 2018)
>         70d8893335b63db006974932341378717c42701c: Changing back the default
> cdn name value (Dylan Souza, Sep 21 2018)
> PR 2812: remove traffic_monitor_java
>         4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove traffic_monitor_java
> (Dan Kirkwood, Sep 12 2018)
>
>
> [truncated]
>
>
> On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:
>
> > Well, our "curated" changelogs are missing A LOT of information on what
> > actually went into the release, which therefore makes it not useful.  If we
> > were better about commit messages and used squash and merge, then we would
> > at least have something for every PR that was merged.
> >
> > On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
> >
> > > > I think this just shifts the burden from writing a changelog entry to
> > > writing a good commit entry.
> > >
> > >
> > > I agree, and I think that’s where that burden belongs. (And I _really_
> > > hate merge conflicts).
> > >
> > > > There might be fewer commit entries if we squash and merge, but I’m
> > > doubtful that it would be as valuable as our “curated” changelogs.
> > >
> > > Both are dependent on how disciplined we are.
> > >
> > > I’m +1 on Squash and Merge.
> > >
> > > Cheers,
> > > JvD
> > >
> > >
> > > > On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > > <ef...@cisco.com.INVALID> wrote:
> > > >
> > > > I think this just shifts the burden from writing a changelog entry to
> > > writing a good commit entry.
> > > >
> > > > There might be fewer commit entries if we squash and merge, but I’m
> > > doubtful that it would be as valuable as our “curated” changelogs.
> > > >
> > > >
> > > >
> > > >
> > > >> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> > > >>
> > > >> Hey All,
> > > >> I want to follow up on something that was briefly discussed at the
> > > summit
> > > >> this week.  Many people do not like the Changelog and feel like it can
> > > be a
> > > >> PITA to deal with.  One of the reasons we have the changelog is
> > because
> > > >> there are so many commits in a given release, that it is hard to comb
> > > >> through all of them to figure out what noteworthy changes or bug fixes
> > > went
> > > >> into the code.  One thing that may help with this problem is to use
> > > enable
> > > >> the squash and merge feature on Github for pull requests [1].   This
> > > >> feature would squash all commits in a PR into one commit.  If we pair
> > > the
> > > >> one commit with a good commit message, we would essentially get the
> > > ability
> > > >> to create the changelog just from the commits.
> > > >>
> > > >> So, I would like to propose that we enable the squash and merge
> > feature
> > > in
> > > >> the Traffic Control Github repo and use that going forward.  Thoughts?
> > > >>
> > > >> Thanks,
> > > >> Dave
> > > >>
> > > >>
> > > >> [1] https://help.github.com/articles/about-pull-request-merges/
> > > >
> > >
> > >
> >

Re: Squash and Merge for PRs

[Jeremy Mitchell <mi...@gmail.com>]

is there a setting in github to enable "squash and merge" vs "rebase and
merge"?

i do have a couple of concerns however:

1. what happens when 2+ people contribute to a PR? This is a pretty common
scenario. If Bob and Sam both work on PR#1 is all the commit history for
Sam lost?
2. i can see benefits of having granular history...to a point...

The more I think about this. Why is squash needed? Commits are grouped by
PRs. Why isn't our change log just a list of PR merged since the last
release? If a PR represents a working unit (as it should), PR titles should
be sufficient, no?

 For example, this is what our internal release change log looks like.

Changelog from 2526569a to 284555703
========================================

These changes are deployed in branch deploy-20181002.

PR 2300: Add TO Go cachegroups/id/deliveryservices
	65e27d2cb7c12b25c97bc98b09ffa6577bb6e1ff: Add TO Go
cachegroups/id/deliveryservices (Robert Butts, May 18 2018)
	130baa85b122f2827621c98f9e87b3245e18d80b: Add TO Go cachegroups/dses
API test, client funcs (Robert Butts, Jun 28 2018)
	5197a2da7e323976d5404ce135ff8c42cbed64ef: Remove TO unused func
(Robert Butts, Sep 13 2018)
PR 2803: Add Traffic Ops Golang Steering Targets
	8ba7d24303d2b420d5059654f83a7154ff7a0703: Add TO Go
steering/id/targets (Robert Butts, Jul 10 2018)
	5ba9a3b40b59f6de2c40ad71c5a0a1a84d3acacf: Add TO API Tests for
steeringtargets (Robert Butts, Sep 11 2018)
	eca0a7eab65cd37d6e4f0658c7640200b2e9ecda: Add TO Go client
steeringtarget funcs (Robert Butts, Sep 12 2018)
PR 2806: Add sub to validate user input when running postinstall script
	e50eeb2c7f46828cbe1d461288f2d98ccdd4ebaa: Add sub to validate user
input when running postinstall script changes default cdn name to not
include an underscore. (Dylan Souza, Sep 11 2018)
	9ac409904987ad0b52e3de26b79422b9688bbb8e: Altering postinstall cdn
name regex to include underscores (Dylan Souza, Sep 21 2018)
	70d8893335b63db006974932341378717c42701c: Changing back the default
cdn name value (Dylan Souza, Sep 21 2018)
PR 2812: remove traffic_monitor_java
	4e7a7ab26cce0903ebe54dd0892da45feaf8d3de: remove traffic_monitor_java
(Dan Kirkwood, Sep 12 2018)


[truncated]


On Thu, Oct 18, 2018 at 9:52 AM Dave Neuman <ne...@apache.org> wrote:

> Well, our "curated" changelogs are missing A LOT of information on what
> actually went into the release, which therefore makes it not useful.  If we
> were better about commit messages and used squash and merge, then we would
> at least have something for every PR that was merged.
>
> On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:
>
> > > I think this just shifts the burden from writing a changelog entry to
> > writing a good commit entry.
> >
> >
> > I agree, and I think that’s where that burden belongs. (And I _really_
> > hate merge conflicts).
> >
> > > There might be fewer commit entries if we squash and merge, but I’m
> > doubtful that it would be as valuable as our “curated” changelogs.
> >
> > Both are dependent on how disciplined we are.
> >
> > I’m +1 on Squash and Merge.
> >
> > Cheers,
> > JvD
> >
> >
> > > On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> > <ef...@cisco.com.INVALID> wrote:
> > >
> > > I think this just shifts the burden from writing a changelog entry to
> > writing a good commit entry.
> > >
> > > There might be fewer commit entries if we squash and merge, but I’m
> > doubtful that it would be as valuable as our “curated” changelogs.
> > >
> > >
> > >
> > >
> > >> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> > >>
> > >> Hey All,
> > >> I want to follow up on something that was briefly discussed at the
> > summit
> > >> this week.  Many people do not like the Changelog and feel like it can
> > be a
> > >> PITA to deal with.  One of the reasons we have the changelog is
> because
> > >> there are so many commits in a given release, that it is hard to comb
> > >> through all of them to figure out what noteworthy changes or bug fixes
> > went
> > >> into the code.  One thing that may help with this problem is to use
> > enable
> > >> the squash and merge feature on Github for pull requests [1].   This
> > >> feature would squash all commits in a PR into one commit.  If we pair
> > the
> > >> one commit with a good commit message, we would essentially get the
> > ability
> > >> to create the changelog just from the commits.
> > >>
> > >> So, I would like to propose that we enable the squash and merge
> feature
> > in
> > >> the Traffic Control Github repo and use that going forward.  Thoughts?
> > >>
> > >> Thanks,
> > >> Dave
> > >>
> > >>
> > >> [1] https://help.github.com/articles/about-pull-request-merges/
> > >
> >
> >
>

Re: Squash and Merge for PRs

[Dave Neuman <ne...@apache.org>]

Well, our "curated" changelogs are missing A LOT of information on what
actually went into the release, which therefore makes it not useful.  If we
were better about commit messages and used squash and merge, then we would
at least have something for every PR that was merged.

On Thu, Oct 18, 2018 at 8:25 AM Jan van Doorn <jv...@knutsel.com> wrote:

> > I think this just shifts the burden from writing a changelog entry to
> writing a good commit entry.
>
>
> I agree, and I think that’s where that burden belongs. (And I _really_
> hate merge conflicts).
>
> > There might be fewer commit entries if we squash and merge, but I’m
> doubtful that it would be as valuable as our “curated” changelogs.
>
> Both are dependent on how disciplined we are.
>
> I’m +1 on Squash and Merge.
>
> Cheers,
> JvD
>
>
> > On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri)
> <ef...@cisco.com.INVALID> wrote:
> >
> > I think this just shifts the burden from writing a changelog entry to
> writing a good commit entry.
> >
> > There might be fewer commit entries if we squash and merge, but I’m
> doubtful that it would be as valuable as our “curated” changelogs.
> >
> >
> >
> >
> >> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> >>
> >> Hey All,
> >> I want to follow up on something that was briefly discussed at the
> summit
> >> this week.  Many people do not like the Changelog and feel like it can
> be a
> >> PITA to deal with.  One of the reasons we have the changelog is because
> >> there are so many commits in a given release, that it is hard to comb
> >> through all of them to figure out what noteworthy changes or bug fixes
> went
> >> into the code.  One thing that may help with this problem is to use
> enable
> >> the squash and merge feature on Github for pull requests [1].   This
> >> feature would squash all commits in a PR into one commit.  If we pair
> the
> >> one commit with a good commit message, we would essentially get the
> ability
> >> to create the changelog just from the commits.
> >>
> >> So, I would like to propose that we enable the squash and merge feature
> in
> >> the Traffic Control Github repo and use that going forward.  Thoughts?
> >>
> >> Thanks,
> >> Dave
> >>
> >>
> >> [1] https://help.github.com/articles/about-pull-request-merges/
> >
>
>

Re: Squash and Merge for PRs

[Jan van Doorn <jv...@knutsel.com>]

> I think this just shifts the burden from writing a changelog entry to writing a good commit entry. 


I agree, and I think that’s where that burden belongs. (And I _really_ hate merge conflicts).

> There might be fewer commit entries if we squash and merge, but I’m doubtful that it would be as valuable as our “curated” changelogs. 

Both are dependent on how disciplined we are.

I’m +1 on Squash and Merge. 

Cheers,
JvD


> On Oct 18, 2018, at 08:18, Eric Friedrich (efriedri) <ef...@cisco.com.INVALID> wrote:
> 
> I think this just shifts the burden from writing a changelog entry to writing a good commit entry. 
> 
> There might be fewer commit entries if we squash and merge, but I’m doubtful that it would be as valuable as our “curated” changelogs. 
> 
> 
> 
> 
>> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
>> 
>> Hey All,
>> I want to follow up on something that was briefly discussed at the summit
>> this week.  Many people do not like the Changelog and feel like it can be a
>> PITA to deal with.  One of the reasons we have the changelog is because
>> there are so many commits in a given release, that it is hard to comb
>> through all of them to figure out what noteworthy changes or bug fixes went
>> into the code.  One thing that may help with this problem is to use enable
>> the squash and merge feature on Github for pull requests [1].   This
>> feature would squash all commits in a PR into one commit.  If we pair the
>> one commit with a good commit message, we would essentially get the ability
>> to create the changelog just from the commits.
>> 
>> So, I would like to propose that we enable the squash and merge feature in
>> the Traffic Control Github repo and use that going forward.  Thoughts?
>> 
>> Thanks,
>> Dave
>> 
>> 
>> [1] https://help.github.com/articles/about-pull-request-merges/
> 

Re: Squash and Merge for PRs

["Eric Friedrich (efriedri)" <ef...@cisco.com.INVALID>]

I think this just shifts the burden from writing a changelog entry to writing a good commit entry. 

There might be fewer commit entries if we squash and merge, but I’m doubtful that it would be as valuable as our “curated” changelogs. 




> On Oct 18, 2018, at 9:40 AM, Dave Neuman <ne...@apache.org> wrote:
> 
> Hey All,
> I want to follow up on something that was briefly discussed at the summit
> this week.  Many people do not like the Changelog and feel like it can be a
> PITA to deal with.  One of the reasons we have the changelog is because
> there are so many commits in a given release, that it is hard to comb
> through all of them to figure out what noteworthy changes or bug fixes went
> into the code.  One thing that may help with this problem is to use enable
> the squash and merge feature on Github for pull requests [1].   This
> feature would squash all commits in a PR into one commit.  If we pair the
> one commit with a good commit message, we would essentially get the ability
> to create the changelog just from the commits.
> 
> So, I would like to propose that we enable the squash and merge feature in
> the Traffic Control Github repo and use that going forward.  Thoughts?
> 
> Thanks,
> Dave
> 
> 
> [1] https://help.github.com/articles/about-pull-request-merges/