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

Decide what needs an RfC #1041

Closed
Manishearth opened this Issue Apr 7, 2015 · 20 comments

Comments

Projects
None yet
@Manishearth
Copy link
Member

Manishearth commented Apr 7, 2015

We should decide on a set of changes that should always need an RfC. I haven't come up with an exhaustive list which I'm happy with (so I'm filing an issue to discuss stuff as opposed to writing an RfC).

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Apr 7, 2015

My opinion:

What definitely needs an RfC:

(I'm only putting objective stuff here, subjectivity will get us back to where we were)

  • New language additions, or tweaks to existing language features, including unstable language features.
  • Changes to stable APIs that can be noticed externally. "noticed" does not include benchmarks. "noticed" includes situations where new functionality is added to an API (without breaking how it used to work), since this is similar to the "stabilization of APIs" point below
  • Stabilization of APIs where the last iteration was not a product of the RfC process. Stabilization PRs must link to the RfC that finalized their design.
  • Changes to the RfC process itself, the code of conduct, or other community guidelines
  • Breaking changes to APIs which are widely used, for some definition of "widely used" which I haven't come up with yet
  • Decisions on guidelines for the naming of functions, the order of parameters, and similar bikeshedding

What probably should use an RfC anyway:

(these are mostly subjective, and hence the points are simply guidelines)

  • Breaking changes to unstable APIs which have a tendency to pull the rug from underneath users (something like this one, even though that was made to stable APIs). Ideally the change should be made in such a way that people who haven't updated to it will be notified by a compile error (either via a temporary lint, or via renaming of functions or something), however such changes should still be discussed.
  • Changes which undertake a major refactoring of an unstable API
  • Changes which may cause significant performance loss in an API or a specific use case of an API
  • Changes to widely-used lints

Thoughts? Neither of these lists seem to be complete to me.

@bombless

This comment has been minimized.

Copy link

bombless commented Apr 7, 2015

FWIW:

New language additions, or tweaks to existing language features, including unstable language features.

Sometimes tweaking existing features is just fixing old flaws.
Plus, after you say a RFC is implemented, and you may later find that you need more things to totally support that RFC.

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Apr 7, 2015

Sometimes tweaking existing features is just fixing old flaws.

Well, we would need some objective guideline to identify these. And generally language changes are breaking, so it's easier to just blanket require an RfC.

@mahkoh

This comment has been minimized.

Copy link
Contributor

mahkoh commented Apr 7, 2015

The people with commit access don't even follow the existing rules. Adding more rules won't change that.

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Apr 7, 2015

What existing rules?

@mahkoh

This comment has been minimized.

Copy link
Contributor

mahkoh commented Apr 7, 2015

The rule they don't get tired reminding everyone but themselves of: Changes to stable APIs require an RFC.

@P1start

This comment has been minimized.

Copy link
Contributor

P1start commented Apr 7, 2015

Dupe of #35?

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Apr 7, 2015

In this case I don't know what happened, but everyone is allowed to make mistakes. Let's not turn this into a slugfest and instead look ahead to see how we can make these rules more concrete.

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Apr 7, 2015

@P1start New issue since times have changed. What required an RfC last year was a smaller subset than now.

@P1start

This comment has been minimized.

Copy link
Contributor

P1start commented Apr 7, 2015

@Manishearth Either this issue or #35 should be closed, then. Traditionally, the earlier issue is kept open, but I'm fine either way.

@mahkoh

This comment has been minimized.

Copy link
Contributor

mahkoh commented Apr 7, 2015

everyone is allowed to make mistakes

Merging this straight away was no mistake. They wanted to get this change in before beta so they disregarded all established rules because it would have been inconvenient. Don't try to spin this any other way.

@Manishearth

This comment has been minimized.

Copy link
Member Author

Manishearth commented Apr 7, 2015

This discussion is going nowhere, and is totally tangential to the original topic. If you want to discuss what happened in that issue, do it somewhere else.

While I was spurred by that incident to file this, I've been confused about the RfC process for a while in this regard and either way such a set of rules is necessary IMO. Let's focus on that, here.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Apr 7, 2015

So some thoughts:

  1. This does seem like a dup of #35 to me :)
  2. I've definitely been wanting to clarify the rules here too. In particular, I think we've been leaning towards too many RFCs lately. It seems too heavyweight for every API addition to go through the RFC process; RFCs ought to be reserved for deprecations and "major" or cross-cutting additions. For other changes, a lighterweight process should suffice (more thoughts on that to come).
  3. Bug fixes which make silent changes to semantics ought to be advertised but do not necessarily require an RFC.

To be clear, I'm referring here only to backwards compat changes -- after all, there shouldn't be any other kind. (The whole "order of arguments" thing seems to me to be a kind of one-time event, and doesn't fit into the above criteria.)

@blaenk

This comment has been minimized.

Copy link
Contributor

blaenk commented Apr 7, 2015

(OT: DUDE @P1start YOU'RE BACK!!!!)

@Gankro

This comment has been minimized.

Copy link
Contributor

Gankro commented Apr 7, 2015

I would love if every change got huge community feedback and productive discussion.

However, in reality most community members don't follow the RFCs repo, and adding more RFCs certainly isn't going to help with that. In my experience most people don't have any strong opinion on API changes until they happen and they run into some pain/confusion and want to bite your head off. Or even if a change gets lots of responses, it's often just a gut reaction without much reflection. Which is fair, really. We're all busy. We don't have time to think about someone else's ideas all day. Best case scenario you get tons of bikeshedding.

Really I feel like most changes are best handled by the staging process. You try something in nightly. If someone notices a problem, you revert it. No harm no foul. We just have extraordinary pain right now because everyone is on the nightly.

@pcwalton

This comment has been minimized.

Copy link
Contributor

pcwalton commented Apr 7, 2015

+1 to everything that @Gankro said.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Apr 7, 2015

Yes, a hearty 👍 to @Gankro . I've been a little quiet on this issue because I couldn't figure out how to articulate it, and that's exactly how I feel.

@reem

This comment has been minimized.

Copy link

reem commented Apr 7, 2015

I feel very similarly to @Gankro.

I used to be able to read basically everything that happened in this repo, but the massive proliferation of both RFC PRs for increasingly small changes as well as hundreds of issues has made that impossible, and I'm pretty active!

@liigo

This comment has been minimized.

Copy link
Contributor

liigo commented Apr 8, 2015

+1 to everything that Niko said
On Apr 7, 2015 11:24 PM, "Niko Matsakis" notifications@github.com wrote:

So some thoughts:

  1. This does seem like a dup of #35
    #35 to me :)
  2. I've definitely been wanting to clarify the rules here too. In
    particular, I think we've been leaning towards too many RFCs lately.
    It seems too heavyweight for every API addition to go through the RFC
    process; RFCs ought to be reserved for deprecations and "major" or
    cross-cutting additions. For other changes, a lighterweight process should
    suffice (more thoughts on that to come).
  3. Bug fixes which make silent changes to semantics ought to be
    advertised but do not necessarily require an RFC.

To be clear, I'm referring here only to backwards compat changes -- after
all, there shouldn't be any other kind. (The whole "order of arguments"
thing seems to me to be a kind of one-time event, and doesn't fit into the
above criteria.)


Reply to this email directly or view it on GitHub
#1041 (comment).

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 11, 2016

We've since grown a lot of language about this, and now we've got a whole README section dedicated to this, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.