31–40 of 210 posts
I agree 1000%. I’m the creator of what I believe is a better review interface for GitHub ( https://codeapprove.com ) but there are also many others: * CodeApprove (codeapprove.com) * Graphite (graphite.dev) * Reviewable (reviewable.io) * Axolo (axolo.co) * Viezly (viezly.com) * Mergeboard (mergeboard.com) * Codestream (codestream.com) * Pullpo (pullpo.io) * ReviewPad (reviewpad.com) * Planar (useplanar.com) * Visibly…
Generally, it feels like a bit of a farce that source code is very well well version controlled, but nothing else is. Data isn't well managed. Isn't version controlled well. The pull request is just another type of data. We can keep improving each applications model. But some day, imo, the general project of computing needs to take data more seriously & develop general tools for managing data over time well & consist…
Earlier quoted context omitted.
Usually you can quickly click through the different changesets and see all the old comments. In my experience, it maybe needs one more click to follow the reference compared to similar comments referencing the current changeset.
Github can show a list of unresolved comments, and with a single click jump to the location in the state of the specific commit the comment was written at. No need to cycle though commits / state at each commit without comments.
Answer, just like with the non-changeset oriented workflow, it depends. If the new PR update deletes the associated lines of code, there is arguably a need for those types of comments to be manually added with the current interface, but not so with a changeset oriented interface.
I like the append only nature of a changeset oriented interface.
But I find this type of conversation quite fiddly to do in pure text without reference to examples and the actual use of both styles, so forgive me if this is still unclear.
I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.
I really don’t see the difference between force pushing and not when you are going to squash merge anyway.
I agree 1000%. I’m the creator of what I believe is a better review interface for GitHub ( https://codeapprove.com ) but there are also many others: * CodeApprove (codeapprove.com) * Graphite (graphite.dev) * Reviewable (reviewable.io) * Axolo (axolo.co) * Viezly (viezly.com) * Mergeboard (mergeboard.com) * Codestream (codestream.com) * Pullpo (pullpo.io) * ReviewPad (reviewpad.com) * Planar (useplanar.com) * Visibly…
There’s also the new open source git thing from meta right
I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.
Here is a random series from DRM patchwork: https://patchwork.kernel.org/project/dri-devel/list/?series=...
Would you squash that? Hell no, of course not. You would be mixing atomic changes in different subsystems. It breaks bisect and makes blames a mess.
Honestly, squash merges are frequently a sign of people who lack mastery in Git itself or make no effort to produce high-quality independent commits.
It'd be a useful thing to import without having to bring in the whole charade of using email and mailing lists (which most mail clients tend to be very unfavorable of in general nowadays) - there's real advantage to doing this in a web interface instead.
UX would probably be more difficult though. The current workflow of "submit branch, make PR, do changes on same branch, merge latest version through web interface" is a big part of the ease of the Github UX. Doing a merge outside of that just by pulling in the right remote branches has always been a crapshoot at best and a pain in the ass at worst. Not helped by the fact that Github's documentation on how to do it in git is obscure (intentional I'm sure; I know it's possible but the docs are scattered and all of it recommend just using their gh CLI tool at this point).
I’m anticipating some push back on this, because I didn’t notice it mentioned anywhere else, even though there are a fair number of comments. So, I may need take some time to understand these other tools. But, short of that, for me personally, keeping things small and relatively easy to understand is the only way to maintain my sanity.
I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.
I really don’t see the difference between force pushing and not when you are going to squash merge anyway.
Relatedly, does GitHub merge understand !squash and !fixup commands? I kind of gave up on those and just accepted a few trailing bugfix commits on PRs to not mess up peoples reviews (some projects also invalidate review on force push).
It's possible that better tooling would help there, of course.
(A surprisingly common pain-point with Gerrit is when you've wound up with a semi-long-lasting stack of patches for some reason, and then you develop a branching tree of sub-patches and need to rebase them all when you make some change higher up. The answer of "don't let a stack last long enough that you need to do that" has an appeal, of course.)