331–340 of 431 posts
Earlier quoted context omitted.
PRs only live on GitHub, what happens if it gets shut down or it accidentally loses some data?
the entire repo lives only on github as well, there is no meaningful difference between git commits and PR comments in a github-hosted repo
Earlier quoted context omitted.
If the commits don't matter why did you make them separate to begin with?
”Updated something tiny and ran CI again until it failed on some other step” Together with backing up your work. Sure you can keep amending your last commit but whenever you detour to another problem in the same PR that turns into a mess. Easier to just treat the PR as the atomic unit of work and squash away all that intermediate noise. It also ensures that CI will pass on every commit on the main branch.
This is why commits are often noise. If people are using commits well, they tell a story. The fact that people often use the tool wrong certainly begs some criticism of the tool, but when used correctly commits are certainly worth looking at one by one
I find the sort of opinions on this post quite common on a subset of engineers - namely mid levels with some time in the career, who start to consider themselves senior engineers and want everyone to follow the same set of strict rules they decided make sense. It’s the same mindset that makes people pedantically apply DRY to every situation or forcing others to TDD basic apps. In practice: - smaller PRs aren’t necess…
My simple suggestion to my teams: PRs are emails to your team and to your future self. Framed in that context it's easier to carry the correct tone and think about scoping / what's important. --- > pedantically apply DRY to every situation I swear DRY has done more damage to the software industry from the developer side than it has done good because it has manifested into this big stick with which to bludgeon people…
This should be commits though. Typically, developers would look for clues in this order:
code -> code comment -> commit message -> PR text -> external document
So commit messages puts the information closer to the user. One hop doesn't seem much, but the time saved adds up as you go.
Also, as some other reader mentioned anecdotally, PRs may not be there forever. E.g. your team may migrate to a new platform PR text and reviews were left behind.
Earlier quoted context omitted.
> yeah for sure you want to squash-merge every PR to main, right? Oh god you're serious? > git is just a tool, and commits are just a means to an end To more ends than you realize, probably, if you put some care in making them
i just don't use commits like you do, and that doesn't mean i'm being less careful or less thoughtful, or that my changes are worse than yours commits are what i say they are, nothing more or less
Ok, good is subjective, I guess, so let's say commits with good descriptions, all the information that could be useful to understand what they do (and where appropriate, why), and a limited and coherent amount of modifications in each; in short, commits that are easy to follow and will provide what you need to know if you come back to them later.
What each color means would depend on the PR, but, for instance, yellow = refactoring, brown = test code, blue = drive-by fix, orange = more efficient data structure etc.
The colors and their meanings could be set by either the author or the reviewers. It would be similar to the file checkboxes that exist today, but in this case, it would be per concept, not per file.
Earlier quoted context omitted.
> I don't even know how could commits only benefit the author; if they're poor they won't help him either, if not as a log of how much work he's done. Intermediate commits are just checkpoints of unfinished code. The author knows that they made them and can revert back to them or use git log --pickaxe-S if there's code they saved to a checkpoint and want to recover. Intermediate commits can have meaningful commit mes…
> Intermediate commits can have meaningful commit messages if the author chooses, but they could also just be labeled "wip" and still be useful to the author. > It's really easy for a note someone writes to themselves to be useful to that person without being useful to other people. After a few months it will probably be as useful to you as to anyone else; if you only use commits as some sort of help while developing…
THANK YOU for saying this. Reading through the discussion, it almost feels that people refuse to put like 3h over a weekend to actually learn git (a tool they use DAILY), and prefer instead to invent arguments why squash merging is so great.
> It doesn't make much sense to place failed attempts in a series of commits (and of their reverts), just go back to the last good commit if something was a dead end (and save the failed attempt in a branch/tag, if you want to keep it around).
I agree that failed attempts are bad to have as code history. If you reasonably split your commits, the commit message has ample space to document them: "Used approach X because... Didn't use approach Y because..."
Earlier quoted context omitted.
PR is the atomic level of work. I'd argue PR-level history (i.e. squash) is often enough and is way cleaner. Why would I care about "commit A", "change parts of A because I misunderstood a requirement", "improve A based on code review" etc.? If I want that granularity, I'd go read the original PR and the discussion that took place.
I always tell my engineers to create atomic commits and we usually review commit by commit. Obviously commits like "fixed review comments" or "removed some left-over comments" or "fixed typo" should not be pushed into a PR you asked others to review. I expect people to understand how to clean their commit history - if they don't I teach them. The senior people who are capable of structured work - e.g. are used to con…
That's not the situation in a normal corporate environment. You want to reduce total time expended (or total cost, at least). It's going to be cheaper to just have a chat with your coworker when a PR is confusing.
Splitting up a PR is a lot of work, especially if you want each commit to compile, but it would be great if you could categorize changes using 5–10 colors and then have checkboxes that you can toggle to hide code corresponding to a color. What each color means would depend on the PR, but, for instance, yellow = refactoring, brown = test code, blue = drive-by fix, orange = more efficient data structure etc. The colors…
How it's going: https://gitmoji.dev/
IMHO, the novelty wears out fast. Especially when your git history starts looking like a Messages thread.
Earlier quoted context omitted.
My simple suggestion to my teams: PRs are emails to your team and to your future self. Framed in that context it's easier to carry the correct tone and think about scoping / what's important. --- > pedantically apply DRY to every situation I swear DRY has done more damage to the software industry from the developer side than it has done good because it has manifested into this big stick with which to bludgeon people…
> PRs are emails to your team and to your future self. This should be commits though. Typically, developers would look for clues in this order: code -> code comment -> commit message -> PR text -> external document So commit messages puts the information closer to the user. One hop doesn't seem much, but the time saved adds up as you go. Also, as some other reader mentioned anecdotally, PRs may not be there forever.…
If you decide to do merges without squashing, then yes, you gotta have to have more hygiene on each individual commit. It creates a lot of unnecessary friction and it's guaranteed to be slower (devs can't use commits as checkpoints/savepoints on their work, but rather each commit becomes a fully fleshed out "intermediate final state"). The only situation where I see this making sense is if you share work on a branch with other engineers (which is also a bad idea).
I find the sort of opinions on this post quite common on a subset of engineers - namely mid levels with some time in the career, who start to consider themselves senior engineers and want everyone to follow the same set of strict rules they decided make sense. It’s the same mindset that makes people pedantically apply DRY to every situation or forcing others to TDD basic apps. In practice: - smaller PRs aren’t necess…
My simple suggestion to my teams: PRs are emails to your team and to your future self. Framed in that context it's easier to carry the correct tone and think about scoping / what's important. --- > pedantically apply DRY to every situation I swear DRY has done more damage to the software industry from the developer side than it has done good because it has manifested into this big stick with which to bludgeon people…
Indeed! I've found many point on this discussion answered by the linux kernel idea of mailing lists where a change is discussed then approved, often with feedback acknowledged