Rename third_party/linux/README.crashpad to .md [crashpad/crashpad : main]
Mark Mentovai (Gerrit)
Mark Mentovai voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Jordan Brown
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Will Harris (Gerrit)
Will Harris voted and added 1 comment![]( "Open in Gerrit")
Related details
- Jordan Brown
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jordan Brown (Gerrit)
Jordan Brown added 3 comments![]( "Open in Gerrit")
Mark MentovaiThis is now blocking better vulnerability coverage for chromium, crashpad and the other repositories chrome depends on. Cross timezone reviews like are challenging, in the future, if you only have nits consider marking as LGTM with nits that can be addressed before submitting.
Jordan BrownThis is now blocking better vulnerability coverage for chromium, crashpad and the other repositories chrome depends on. Cross timezone reviews like are challenging, in the future, if you only have nits consider marking as LGTM with nits that can be addressed before submitting.
I sense that this is frustrating for you. I apologize, but allow me to explain.
I can’t LGTM if I’m not convinced that a change won‘t land without uncorrectable defects.
A misleading or confusing first line of a commit message becomes a permanent part of the git shortlog. If a change lands with a defective first line of its commit message, it’s an uncorrectable defect.
A commit message devoid of context altogether is also an uncorrectable defect, and that’s how this change was originally proposed, both in the predecessor change https://chromium-review.googlesource.com/c/6966151/ where I first asked for better commentary, and in the first upload of this change at https://chromium-review.googlesource.com/c/6975426/1.
Why is it uncharacteristically important to get it right the first time in a commit message? Contrast it with code: if you goof in the code itself, or in a comment, sure, you can fix it up in a follow-up, no problem. The intermediate state of the code, although perhaps imperfect, is ephemeral. We trust each other to land a follow-up to correct those problems, right? Commit messages are different: the commit message isn’t ephemeral. It’s a fixed, permanent description. If you goof a commit message, there’s no opportunity to recover. We won’t rewrite git history, and although you might consider landing a follow-up with a better message, it’s not going to be obvious to anyone, including existing tools, or be a part of anyone’s workflow, to look at nearby changes to try to understand the context of one with a defective commit message. If something lands with a bad message, you‘ve doomed all future engineers—yourself included!—attempting to make sense of the change. That’s not a fair position to put your coworkers or future self in.
Cross-time zone reviews _are_ challenging—for your reviewer too!—and if you want to help meet that challenge (really, a good idea even when things don’t need to circle the planet), please consider review feedback more diligently, and perhaps even approach your own changes as a reviewer and not just author. As part of that, I recommend that you give thought to long-term durability of your work product, commit messages included.
As someone tasked with cleaning up a lot of bad metadata, I sympathize with wanting people to uphold a certain standard. I spend a lot of time reading through poorly written commit messages, confusing metadata files, and breadcrumbs like this very file. Dealing with hundreds of these tiny changes, I also sympathize with balancing this with productivity.
Rename README.crashpad to .mdJordan BrownThis repository is full of README.crashpad files. In this change, you’re only working on one.
In fact: is it even correct for you to only work on one? Are any other README.crashpad files going to be a problem for your licenses.py? If so, please just deal with them all together at once.
Mark MentovaiThis is the only problematic one.
Jordan BrownThis is the only problematic one.
Then say so in the commit message.
The commit message isn’t just a scratchpad to communicate ideas with your reviewers at review time. It’s at least as important to leave a long-term and accurate record of the change and what motivated it.
I think our main difference here is actually what we consider 'sufficiently apparent' in a given change, and I will remember this in future changes.
R=w...@chromium.orgJordan BrownPlease don’t use R= lines anymore. Gerrit will automatically insert the appropriate review information such as Reviewed-By: into the commit message’s footer.
Mark MentovaiI used `git cl upload --r-owners`, I can file a bug for you if you'd like? What component does git cl belong to?
Jordan BrownYou can peruse https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/refs/heads/main/git_cl.py. It looks like Chromium→Infra→SDK.
Acknowledged
Related details
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jordan Brown (Gerrit)
Jordan Brown voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Crashpad LUCI CQ (Gerrit)
Crashpad LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Rename third_party/linux/README.crashpad to .md
'README.crashpad' is recognised by our tooling as a metadata file. This
invalid file prevents us enforcing vulnerability coverage. There are
about a dozen valid 'README.crashpads, see examples of correctly
formatted ones by searching on codesearch.
- R third_party/linux/README.md
Code-Review: +1 by Will Harris, +1 by Mark Mentovai
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |