-
Notifications
You must be signed in to change notification settings - Fork 1
Motivation refactor based on feedback + new quote #32
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
Conversation
Signed-off-by: Michał Górny <mgorny@quansight.com>
To make it easier to edit sections without having to move initial definitions around. Signed-off-by: Michał Górny <mgorny@quansight.com>
These additional projects are listed without any explanation, so let's just defer to specific examples in the "current workarounds ..." section. Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
This roughly repeats the introduction to motivation, and focuses on solution rather than the problem. Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
rgommers
left a comment
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.
Thanks @mgorny. Overall this looks like a significant improvement.
The one change I'd like to discuss is moving the "Out-of-scope features" paragraph to Rejected Ideas. I quite like that paragraph and I think it's important for a PEP to articulate what its scope is and isn't. Also, we never intended to standardize those things, so it's not actually a rejected idea I think. I don't see a review comment on that section, so I guess this change was in response to the high-level comment about reducing the Motivation section? I do agree it doesn't fit perfectly in Motivation; one place where I think it could fit well is at the end of the "Summary of changes" section at the top of Rationale (without a separate section header). WDYT?
Actually, it was explicitly requested in one of the comments. But I do agree it's a weird fit, so I'm fine with moving it elsewhere. "Rationale" is already being refactored, so I'd rather leave it in its previous location for now, and take another look when both PRs are merged. |
👍🏼 SGTM |
This reverts commit 261704f. Signed-off-by: Michał Górny <mgorny@quansight.com>
rgommers
left a comment
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.
I'll approve, thanks Michal! I'm not sure if we're waiting for others to have a look or not, so I'll avoid hitting the green button now.
|
Let's merge it now. We can do more PRs later if need be. |
No description provided.