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 <[email protected]> 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 <[email protected]>
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 <[email protected]>
> 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 <[email protected]>
> 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 <[email protected]> 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 <[email protected]> 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)
> >>>> <[email protected]> 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 <[email protected]> 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/
> >>>>>
> >>>>
> >>>>
> >>>
>

Reply via email to