> > After a `git fetch` or a `git pull` it is very easy to review what > > the change is or diff it against the current main branch. There are > > a bunch of tolls, including of course the usual gitk and `git > > difftool --dir-diff` + Meld. > > The problem is that it's also very easy *not* to do a very careful > review. And this is where I really object to the pressure campaign to > tell package maintainers that they have some obligation to review pull > requests from new contributors post-haste, or they are a bad, bad > person.
The above is very subjective language, I should probably not continue this thread but.. > > Indeed, Forge's don't work offline, but you could git pull/fetch the > > branch before you board an airplane? And then after review git merge > > locally? And once you are off the flight run git push and all modern > > Forges will automatically detect that the commit landed on main and > > close all related pull requests and issues. > > Reviewing all of the changes via a "git diff FETCH_HEAD" is far more > difficult and much easier to miss things than reviewing each patch A person with your seniority surely knows better commands to use than review plain diffs. Naturally, it is better to review using the git log -p output that shows each commit message individually and related changes. Or using gitk graphically. In fact, as described in https://optimizedbyotto.com/post/how-to-code-review/ I would always start by reviewing the commit message to understand *why* the change is being done before even looking at the actual code changes. What you see in email as patches are generated in git and you for sure know that the exact same output can be generated offline from git commits by a plain git show. > If someone sends me a gargantuan commits with dozens of logical > changes, whether it's in a pull request or a singleton message, I > *will* just reject the change outright or just ignore the change until > I have a huge amount of time. Same. > This is why e-mail review of dozens of patches is just far more > convenient, and more secure, than doing a git pull and then trying to > review the damage using "git diff FETCH_HEAD" Sure it is more convenient to, you as you most likely have some well optimized Emacs email setup going on. But more secure? Surely signed commits and a system that tracks real git commits and who pushed what from where is more secure than plain-text patches in e-mail. Note that I am not claiming that the current UX of e.g. GitLab is perfect. There are many things to improve, for example the default MR review vies should show all commit message as an expanded list instead of hiding them. My point here is that your e-mail setup is something you have optimized for years. I don't think it is something the next generation of devs is going to adopt. Unless of course git itself would start shipping an email client that has built-in the features to automatically show colorized diffs and arbitrary diffs against any selected target branch, and diffs between diffs, and fetch CI statuses from external APIs to render in the email client view etc. It is much easier to build an integrated UI in a browser that has full audit logs and discussion tracking etc with an git API, than it is to build a git client with all of that and an e-mail API.
