+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_g...@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_fi...@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 <rawlin.pet...@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 <neu...@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 <mitchell...@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_geli...@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.pet...@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 < > mitchell...@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 <neu...@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 <j...@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) > > > >>>> <efrie...@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 <neu...@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/ > > > >>>>> > > > >>>> > > > >>>> > > > >>> > > > > > >
