Consider not squashing commits when merging, to support contributors

[aufdenkampe]

We understand the reasons commonly argued for squashing commit history when merging.

The practice of squashing history, however, makes it much more burdensome for any contributor who is working from a fork, such as ours: https://github.com/LimnoTech/pygeoapi (used to deploy https://api.water.usgs.gov/). This is because it requires tedious manual investigations to figure out which, if any, of our earlier pull requests have been incorporated in each official release or the main branch. Then, once we have figured that out, we need to rebase some some, but not all, of our branches to align with the current state of the upstream repo. This is a lot of tedious work that we believe adds needless friction to contributing to this project.

The alternative, not squashing commit history when merging, substantially streamlines all of that work, because commit tags and history are preserved among all forks and branches. We are not alone in recognizing that squashing history is harmful. For example, see https://dev.to/wesen/squash-commits-considered-harmful-ob1

@tomkralidis, @webb-ben, @kalxas, please consider changing your practices around squashing history. The community of contributors will appreciate it.

cc. @dblodgett-usgs, @ewojtylko, @ptomasula, @mikemahoney218-usgs

[tomkralidis]

@aufdenkampe thanks for the information. We can discuss at the next pygeoapi meeting, however pygeoapi squashes PRs to be able to safely revert and backport commits in a concise manner. Perhaps squashing from your fork before / during a pygeoapi PR can be helpful.

[aufdenkampe]

@tomkralidis, thank you for considering our suggestion!

There are other benefits to not squashing history, described in the post I linked above, including an argument that squashing actually makes back-porting and bug-sleuthing more difficult.

The idea of squashing our commits before issuing a PR might not really solve our problem if you continue to squash commits, because our commit tag will be replaced by yours. Not squashing commits preserves our commit tags.

We look forward to hearing what the core team decides.

[francbartoli]

@aufdenkampe this is just to argue my -1 on the above request. It’s not clear to me why pygeoapi should change some of its practices to make the workflow easier for a downstream project that wants to maintain a custom fork and not the other way around.
As of matter of fact, your proposal would make our maintenance workflow way more complicated and the usability of the history definitely worse.
Perhaps you could start using pygeoapi as a library and maintain your custom code on top of it?

[dblodgett-usgs]

Hi All --

I feel I should speak up here in defense of @aufdenkampe -- he is representing a very real issue that we are experiencing in several production deployments of pygeoapi. Our teams are doing our best to contribute bug fixes and important features to pygeoapi and we've had more difficulty managing our git history on our contributions than feels necessary. Perhaps we are not seeing things from each other's point of view or perhaps this is just a really hard problem.

Either way, I would appreciate it if some specific rebuttal to the points offered in Anthony's cited post would be provided before downvoting and telling us we are wrong and we should just use the library.

My read is that the best practice here is for developers to take care in crafting a clean commit log that tracks compact and cohesive blocks of changes rather than using PR-level squashed-commit policy to achieve the same goal. The iteration cycles / PR review and release timelines are long and convoluted enough that we inevitably have to move forward with changes destined for pygeoapi core (especially bug fixes) in our production deployment or we end up in a protracted "waiting for others" blocked state.

To be clear, we do use pygeoapi as a library ... one that we contribute to and use in production.

So my proposal would be to expect PRs to have a compact commit history and that part of PR review could require a person to rebase a commit history that doesn't match geopython convention.

Our other option is to maintain a stronger separation between our forks and the upstream library code and just move much slower in general. I don't think that's necessary and it would feel like a real shame to give up agility that we could achieve through a slight change to the way the community tracks change.

Definitely work discussion at the next community call. I appreciate everyone engaging on this -- it's definitely in the category of "sticky open source problems that will never go away".

Regards -- Dave

[francbartoli]

Hi @dblodgett-usgs,

thanks for jumping in the discussion, it is worth considering other points of view. However, as a member of the project's governance I do believe the effort here is to preserve the speed’s pace of our team and its preferences in the developer experience.
I don’t think there is a best practice here but just a preference and my -1 stands for my own as I normally do use squashing.
Also, when I make some reviews for medium/big PR, I tend to focus on the design rather than on ephimerel changes to the history.
Again, without any criticism on your point, I believe it’s just a priority for the project.
Anyway, if the PSC wants to explore this request you could open a RFC with the proposal and reasons to enforce this workflow so that we can formally vote for it. To be honest, you shouldn’t expect I would express a different vote for that, sorry.
Besides that, I agree on the fact that a PR should have a compact commit history as a general rule but I disagree to spend our time to rebase or enforce this convention for no real benefits (over a squash action at the end of a review) in the pygeoapi’s governance.

To be clear, we do use pygeoapi as a library ... one that we contribute to and use in production.

if you are using pygeoapi as a dependency and pinning it either to a master's commit or a tag what's the issue with the history? Sorry but actually I don't understand how a pip install pygeoapi workflow and the pygeoapi history would affect your downstream projects.
I would expect you could extend and customize pygeoapi at certain level, would be a valid alternative instead of having a fork with your current git history workflow?