Skip to content
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

updating with new JEP process #52

Merged
merged 3 commits into from
Jun 12, 2020
Merged

Conversation

choldgraf
Copy link
Contributor

This is one step towards implementing the meta-JEP from #29

It does the following:

  • Updates the enhancement proposal guidelines with text that was taken from the PR
    • I made minor modifications for structure and cohesion, but none that should have changed the intent or meaning of the meta-JEP.
  • Adds a new JEP proposal template. This is heavily-inspired by the Rust RFC template
  • Adds a PR template to remind people to follow the JEP process.

Happy to take comments or suggestions!

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good overall -- a few small comments inline.

|
+--------------+ +-----------+ +-----+-----+ +-------------+ +-----------+
| | | | | | | | | |
| pre-proposal +---> submitted +--------------> RFC/FCP +-------------> in progress +---> completed |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Perhaos make this RFC (Request for Comments) for people who might be unfamiliar with the term.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 from me, OK if I leave out the "FCP" in that case, to save space?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep -- sounds reasonable.

```
+-----------+
| |
+-----------+ | withdrawn |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an arrow that takes you from one (or more) of the steps to withdrawn?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I wondered about this too - I wasn't sure how to do this without having a bunch of arrows cluttering up the viz. Maybe we can have a "floating arrow" that doesn't come directly from one box to make it clear that at any point it can move into "withdrawn"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Floating arrow works. Or and endnote in the document that lets people know you can move from any state to withdrawn. Endnote seems like it would be the least obtrusive and most informative.

@choldgraf
Copy link
Contributor Author

OK, just added a commit that addresses both of your comments. For the "withdrawn" section, I added a short note in the diagram, as well as note at the beginning of the "phases" section of the document

@captainsafia
Copy link
Member

Sounds good -- thanks.

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing on this!

@choldgraf
Copy link
Contributor Author

Hey all - comments addressed. Ready to merge from my perspective (note this should be seen as one PR implementing JEP #29 , and more can follow if there are other things to improve), or happy to address more comments!

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@blink1073 blink1073 merged commit 0739b61 into jupyter:master Jun 12, 2020
@choldgraf
Copy link
Contributor Author

wohoo! thanks all for your comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants