Lint: Update headers and checks per current guidance & provide helpful feedback#2484
Conversation
hugovk
left a comment
There was a problem hiding this comment.
I still have some concerns about adding email addresses without permission, and replacing user at example.com with [email protected], but everything else is fine, and I won't block merging over it. Thanks!
Well again, the only email addresses added in the current version of the PR were those for same core devs who already chose to include them the headers of their other PEPs, and which are all already listed in the Authors Index, so it is only making things consistent between PEPs. Also, I'd initially thought from PEP 12 that the email address was a required element and had the checks enforce that for new PEPs, which is why I'd initially made the change, but I'd then realized after cross-referencing PEP 1 that it wasn't, and updated the checks accordingly. Much the same is true of In both cases, there is little benefit conforming the existing source headers when neither is actually required; its just that likewise, it would also be a fair amount of work for no clear benefit to reverting them either, since the emails are already listed and obfuscated. But as it seems its the one thing that's blocking this PR, I guess I'll just bite the bullet and spend the time doing it. |
|
All done—this PR now does not add any existing emails, nor does it conform them to the standard address format. Also fixed what would be a conflict with #2531 , clarified the check feedback a bit further, and a couple other minor refinements. |
hugovk
left a comment
There was a problem hiding this comment.
I appreciate all your work here, thank you!
|
Actually, I noticed there's actually a significant problem as it stands allowing manual obfuscation—glancing at the However, since emails with manual obfuscation applied don't get converted into reference nodes, the automatic obfuscation logic doesn't work, so these emails actually are substantially less obfuscated than they would otherwise be (which PEP authors are almost certainly unaware of, and wasn't even to me until I dug deep into the code and actually tested it). Picking on myself here, from PEP 0, compare these two: <tr class="[row-odd]()"><td>Gerlach, C.A.M.</td>
<td>cam.gerlach at gerlach.cam</td>
</tr>versus, e.g. <tr class="[row-even]()"><td>van Rossum, Guido (GvR)</td>
<td>guido at python.org</td>
</tr>I attempted to add support for also properly masking manually obfuscated emails, but after some testing realized it would require some pretty significant code changes due to how things are currently structured, as well as some hacky and potentially unreliable heuristics. Furthermore, I realized this also makes the not showing the email addresses all, or only as abbreviations, as discussed in #2514 more complicated, since whether or not the email is actually processed as an email changes the doctree structure and node types. Therefore, I conclude it would be best to re-revert the part of the previous change to conform the small minority of emails that were manually obfuscated to use standard email syntax and restore the linter check for such, so they are correctly processed and masked by the header transform code (and anything else that needs to mask/obfuscate/elide them), in order to ensure that the various automatic measures to protect authors' emails work consistently. |
|
Actually, upon giving this more thought, making the author-emails As such, I suggest we just go ahead and merge this PR as-is with the manual obfuscation still untouched, and then I can address properly masking manually "obfuscated" emails along with formatting them consistently as |
|
Since it seems I've satisfied the immediate concerns that were raised and the two PEP editors that previously reviewed have ✔️ ed, this PEP has been open for a while and it seems there aren't any further objections, and it is blocking some further discussed and agreed changes ( |
Over the past >year, we've made significant progress in programmatically parsing the PEP headers, using them to intelligently display more useful, informative and readable output in the rendered PEPs, conforming them to our modern guidance, automatically checking their format and (particularly now with #2475) making it easy for other tools to consume and enrich them.
As a final step toward these overarching goals, this PR:
Overall, this PRs enhancements to our existing checks:
To note, this PR does not make any new headers required, invalidate any existing established header formats nor require any of the newer ones; the new checks only trigger on the formats.
In addition to the above, this resolves the linting side of #2402 and supports the improvements in #2475 and #2467 .