Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

But again, the age-old response to this flow is that it creates an untrue version history. This is great if cleanliness is your exclusive priority, but if you find that you need to actually try to jump into the mind of the person who made the commit (especially if it's someone you've never met and with whose habits you aren't familiar), it's impossible to actually follow along and recreate what really happened.

The rebase strategy and merge strategy both have their places - neither is fit for "whenever we do a pull request" for all people everywhere.



I don't follow because the rebase is done by the person who wrote the code and is proposing the pull request. They rewrite their local history, but that doesn't seem to relevant. They still have to get their code to work with the latest master and remain in control of it. When the PR goes through, it can be a standard merge to maintain history.


True, but while that person probably tested each original commit as they wrote it (at least to the extent the application compiled and didn't completely blow up), they probably didn't do the same for each rebased commit every time they updated to master. If the resulting history turns out to be broken, it isn't the end of the world, but makes bisecting harder.

I think rebasing is a good idea anyway, but it's a tradeoff.


There's no perfect solution and all of this still requires a fair amount of vigilance to get it right. But I would still argue that if you are proposing a PR, you've done the work to verify your changes work, tests pass, etc. If you later need to rebase your PR, you still should run tests, smoke test, etc.

Small issues can slip through, but on teams I've worked on, submitting a PR that is truly broken is really bad form. Sometimes that does mean submitting PRs is tedious, but that's just how it goes sometimes. Thankfully, it's not that often in my experience.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: