search

zopefoundation / persistent

automatic persistence for Python objects
https://pypi.org/project/persistent/
Other
46 stars 28 forks source link

Drop support for Python 3.4, add support for 3.8. #110

Closed icemac closed 5 years ago

mgedmin commented 5 years ago

Heads up: I dropped Python 3.4 support in #111 for reasons, which may have introduced merge conflicts with your PR.

I apologize for the inconvenience!

mgedmin commented 5 years ago

Can you please rebase so there are no merge commits in the middle of the PR?

icemac commented 5 years ago

My plan was to squash merge this PR to get rid of the back and forth (e. g. with Terryfy and Python 3.8). Rebasing requires a force push and I already got negative comments for using force pushes in PRs because it makes it hard to follow the changes. I even changed the settings in the Zope project to only allow squash merges. This means everything can happen on a branch (GitHub allows to merge from the destination branch with one click!) but there will only be one clean commit on master.

What do you think about this?

mgedmin commented 5 years ago

Sure, a squashed merge seems fine!

jamadden commented 5 years ago

My plan was to squash merge this PR to get rid of the back and forth (e. g. with Terryfy and Python 3.8). Rebasing requires a force push and I already got negative comments for using force pushes in PRs because it makes it hard to follow the changes. I even changed the settings in the Zope project to only allow squash merges. This means everything can happen on a branch (GitHub allows to merge from the destination branch with one click!) but there will only be one clean commit on master.

What do you think about this?

Speaking for myself, I'm fine with occasional judicious use of squash merges, but I do not like the idea of forcing that as a policy. There can be lots of useful information in commits, even if those commits are later reverted. Knowing how a particular state was reached, what was tried and what wasn't, can be even more useful than just knowing the end result. In an ideal scenario, that information winds up recorded in new comments in the files themselves (requiring new commits), but too often a sequence of commits "do abc" and then "revert abc because it breaks xyz in fascinating way 123" just winds up squashed out as a no-op and the information that abc was tried, it broke xyz because of 123 is just lost.

If a branch has a particularly messy commit history with lots of useless thrashing, sure, rebase it and squash some of those individual commits into coherent units as a final step requiring a force push. But don't throw away information.

mgedmin commented 5 years ago

Rebasing requires a force push and I already got negative comments for using force pushes in PRs because it makes it hard to follow the changes.

FWIW my perspective on this is: