191–200 of 278 posts
Earlier quoted context omitted.
Seems like you can add —no-edit and get the same behavior, now I can delete that alias. Thanks again :) (Edit: typo)
That is correct, and there is a `--edit` to revert that, so my personal alias is to `git ci --amend --no-edit` such that by default it just merges the staging into the HEAD, and then tacking on `-e` will open the commit message in an editor to expand it.
Maybe I am being to much of a purist, but retroactively modifying commits and history? Why? Stuff happens, so do mistakes. Fix the mistakes, make another commit, and go on with your life.
Do people actually check commit history in detail so often that they absolutely find so much value in ultra clean commit history? I never understood that obsession with 100% clean history.
Earlier quoted context omitted.
It's hard to follow your example. You say "go from red to green" which I read as "go from failing to passing," but then you go on to say "A becomes A'" where "A is good, but A' is not." Either way, the answer is that if that commit is in your patch series that hasn't been merged yet, that it might make sense to just rewrite A'. But it might not. It could be a ton of work. But also, 20 commits in one branch is rather…
Yeah, that's what I meant. I guess at the end it's all weighted tradeoffs for me too. I just put less weight on legibility and more on the ability to work on branches co-operatively without force-pushing.
And this is also why workflow questions are hard. Because "working on branches co-operatively" was previously unstated. It's not universal. I rarely work on branches co-operatively in a long term sort of way. Sometimes I might "take over" a branch to get it merged or similar, but rewriting history in that context is still fine.
It is always the case, even among so-called purists, that you don't rewrite history that you're sharing with others. (Unless you've both agreed to it. But even then, it better be short-lived because it's annoying.)
Uhm, I do a lot of git rebase -i HEAD~2 where I just squash the commit on the latest or sometimes I need to reorder and move the fix commits in specific commits in Pars that multiple commits, which I then need to push force. Is this for a similar use-case? I am not familiar with fixup or how it works.
This sounds great,but kind of an anti pattern in git. I definitely want to have a "fixes" commit on my feature branch. You should do whatever you want on a feature branch so long as your trunk has a clean history. This sounds like someone wanted to lift a feature of changesets in mercurial into git. I don't think this is safe and probably breaks a lot of people's mental model of git changelogs being an immutable data…
Earlier quoted context omitted.
At some point the changes are going to get merged in, no? And that that point I would really like the commits to be nice.
That's why you squash and make the commit message readable.
Earlier quoted context omitted.
Can you give me an example of criticism that is not negative? As far as I know, all forms of criticisms involve pointing out a flaw or fault. There's constructive criticism, but it's still fundamentally negative. Either way, feel free to replace the word "negative" with "criticism" in my comment if you want. It expresses the same thing I intended to express: I disagree with the criticism. If we're not Pollyannas and…
I think GP is saying that you didn't need to focus on the negativity (which is in itself negativity), just say the substantive thing that you wanted to say without editorializing about negativity. Your complaint (negativity) about negativity might be over-done, and anyways not useful. We can all shake our own heads at others' possibly-unnecessary negativity without having to be calling it out all the time. Meta argum…
I even already said I would use different wording next time.
And I never said folks shouldn't make those comments. I clarified that too.
As a frequent user of fixups, this feels like a solution for already broken workflows. > Instead of manually finding commit SHAs for git commit --fixup Assuming you are using fixups, is this actually a problem? I could see this being a possibility if you are: A. not practicing atomic commits or B. have so many commits in your branch that this is a chore. A. seems unlikely if you are already using fixups and B. seems…
Earlier quoted context omitted.
What are your thoughts on the "ship, show, ask" workflow? [1] In that workflow, small stuff is simply pushed, which allows PRs to be more single focused and more atomic. Perhaps your only objection is direct pushes to master? I am really curious if that workflow otherwise addresses all of the downsides you stated while still allowing for all PRs to be uniformly rebase-squash merged. [1] https://martinfowler.com/artic…
I wasn't aware of this particular workflow, but I've heard of similar ones before. The late Pieter Hintjens, of ZeroMQ fame, advocated for a strategy called "optimistic merging"[1], which essentially abandons the standard code review process in favor of merging changes ASAP, and fixing any issues as they arise later. I'm not a fan of this. It allows contributors to abandon any established coding standards, while plac…
First, I assume a few constraints on "ship, show, ask." First constraint is that all changes go for at least one, if not multiple self reviews. Second, the ability to ship is not outright granted. Until someone has demonstrated they can ship reasonable, working code- they are not given write permissions. They must "ask."
Next, ship is not a given. Things that are not interesting are shipped. This enhances the communication part of PRs. If a PR is sent, it is important or wants extra review. That removes the noise of everything being a PR, and removes the noise from PRs where minor changes are smuggled in for the sake of efficiency. This also removes a lot of nitpicking as well. When asked to CR, we often feel we need to CR, and if our only feedback are minor things- then that is the feedback. 'Perfect' becomes the enemy of good enough. Saving time, efficiency are huge. Do CR when it is useful, and only then.
Last, post merge reviews can still be done by the maintainers to ensure things are going smoothly. Hence, post merge review is an option for both ship and show. This means the coding lifecycle is blocked in CR only when the CR is valuable.
That CR is blocking I feel is a very important drawback. Typos linger because time is finite and only those that do not appreciate that will try to fix everything. The team then gets bogged down spending too much time fixing small things. Leaving the small things is not ideal either. If the overhead to fix small things is tiny, suddenly it takes 5 minutes to do what would otherwise require another person and possibly the next day.
For example, A PR preceded by renaming a few files is powerful. That is a game changer compared to a hard to review CR, staged PRs, or simply never doing the renaming because it runs the diff.
Blanket CR can also ossify a project. Most projects I've seen are written quickly and in large chunks. If they are reviewed at all, it is a lot of 2000 line "LGTM", zero comment reviews. Then when careful CR is done (often by the "scaling" team), suddenly things that are original become gospel. The CR debates the absolute best naming, original intent, spending magnitudes more time than was originally put into the code in the first place. Meanwhile the code was done quickly, slightly worse than good enough- and now good enough is not even good enough. The CR now demands a far higher quality bar compared to the original code when it was written. Fixing a 50k line code base at 500 lines per day is a way to _not_ fix that codebase. The original bad code is therefor fossilized into the project. Time and effort are not infinite.
That turned into a bit of a rant. I wanted to demonstrate that:
(A) there are guard rails on SSA so it is not just a free for all.
(B) selectively requesting CR when it is valuable amplifies the value of CR.
(C) removing the noise of everything is a PR makes PRs stronger as communication tools.
(D) post merge review of all code can still meet the goal of reviewing everything.
I'll tie this back to git absorb now. Git absorb rebases, which arguably invalidates any CR that was done. Ship-Show-Ask allows for there to be more explicit trust where someone does their git absorb, redoes the self review and then merges. Further, the workflow helps clean up PRs to not be as many commits.
Though, I do think the discussion of workflows and CR is really fascinating. Having been essentially just the one senior, or one of two on most all projects I've worked on in the last decade, - I'm certainly (overly) eager to discuss in detail the workflows.