-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PEP 1: Further refine phrasing and organization of PEP-Delegate description #2257
PEP 1: Further refine phrasing and organization of PEP-Delegate description #2257
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks. A few suggested edits.
(Just FYI, @willingc , since many devs don't realize this—GitHub allows you to do multiline comments and suggestions by dragging your mouse to the left of the lines when the blue plus appears, which is really handy for logical changes that span multiple physical lines like this one). |
FYI there's a merge conflict now. |
ef848de
to
c03489b
Compare
@JelleZijlstra Thanks, fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of suggestions
Co-authored-by: Jelle Zijlstra <[email protected]>
This change has introduced inconsistency into PEP 1. Paragraph 4 of PEP Review & Resolution says:
Paragraph 7 says:
I ran into this whilst writing up the PEP 676 delegate notification to the SC. Am I meant to ask for explicit assent of the PEP-Delegate, or simply not a veto? A |
Let's see what the SC has to say on your linked message and update this PEP accordingly. (Also, happy to see Barry as the delegate for PEP 676!) |
I believe the key confusion here is between the SC's decision-making philosophy on PEP-Delegate nominations, which is accept-by-default (paragraph 7), and the notification procedure for PEP-Delegate acceptance (paragraph 4), which includes explicit notification of acceptance from the SC like any other SC ticket (otherwise, it creates a high potential for confusion, as offerers might assume after some unspecified period that they were accepted, upon not receiving an explicit notification of rejection over whatever channel they chose, rightly or wrongly, to communicate with the SC). For some background, these changes were in response to #1430 (and @willingc 's original #2256 merged to address it), which in turn was prompted by @gvanrossum 's request for clarification on the SC acceptance process in python/steering-council#28 , where on behalf of the SC, @warsaw confirmed the former's appointment as PEP-Delegate by explicitly approving it, stating
The original paragraph 4 wording prior to this PR (which was unchanged in #2256) didn't explicitly mention that the Steering Council could veto a PEP Delegate, or indeed how one was confirmed at all, instead seemingly implying it was automatic and immediate (contrary to paragraph 7, which was essentially untouched by both PRs):
Further details on how we got here: My original change in e200328 added wording that matched paragraph 7,
@willingc then suggested. a further clarification, adding the procedural note "and they should notify the Steering Council of their offer"
At the time, given this was already discussed more specifically in paragraph seven, which discussed procedural matters, as opposed to the basic summary of the role of a PEP delegate in this paragraph, I elected to modify this somewhat to "assents", in my resulting revision c03489b , to attempt to make clear that the SC had to actively receive the notification, consider it, and respond accordingly (i.e., formally state on the GitHub issue that they accept, or otherwise):
To note, @willingc mentioned a related concern with this in reply,
However, as the comment was marked resolved, I didn't make further changes before the PR was merged. To both clarify the situation further and address @willingc 's further suggestion, I suggest something like this for paragraph 4:
And slightly revising the above-quoted first sentence of paragraph 7 to read
This makes it more clear that the SC does need to explicitly state (i.e. on the relevant SC issue) that the nomination is accepted, while their decision is to accept by default, if a suitable reason is not present to decline. EDIT: [Forgot to add this] That being said, I'll wait to see what the SC says before going ahead with a PR to that effect. |
@CAM-Gerlach Nice job restating the intent of the SC when I wrote the original PR. Thanks for your work on this. @JelleZijlstra @AA-Turner Thanks for being so helpful and thoughtful on these documents. |
Thanks for your very kind words, @willingc . Your feedback and suggestions were crucial in helping bring this discrepancy to light and point to a path forward, not to mention it was your previous PR that we're building upon here. I really should have given you more explicit credit in the somewhat coldly clinical summary above, as well as point out that had I carefully considered your thoughtful review comments before pushing ahead with this, we might have ended up with wording closer to the above before merging the PEP. |
No worries @CAM-Gerlach. Iterating is a good thing. Thanks for shepherding it to merge. |
Further refine the PEP 1 phrasing around PEP-delegates, as originally opened as #1445 and further reviewed and merged in #2256 . See my review on #2256 for the rationale behind the specific copyedits (which I've further refined since then).
Also, fixes a related instance where
python-dev
instead of the Steering Council was specified as the final approve of PEPs (I plan to open another issue to discuss also mentioning/linking the PEPs category of the Python Discourse, as well as a few related changes).