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

Update RFC numbering guidance to use PR number #17

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

areusch
Copy link
Contributor

@areusch areusch commented Aug 4, 2021

Hi all,

After some discussion, we'd like to propose that RFCs are numbered according to the GitHub PR number. This follows the idea of documenting those RFCs which are rejected. While it adds an extra step to the initial RFC posting, it simplifies all future references by not allocating two different IDs (PR number and RFC number). It also makes it very easy to find the discussion associated with a given PR when viewing it outside of a PR (e.g. through browsing the tvm-rfcs repo) and makes for one less link to update.

This also has precedent in the rust community.

Sending this PR as a proposal and would like for everyone to discuss/approve/reject this idea here.

@hogepodge @apache/tvm-committers @tqchen

-Andrew

@tqchen
Copy link
Member

tqchen commented Aug 4, 2021

I like the idea. It follows the common practice in previous communities and simplifies the RFC process.

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

Agree. It makes a lot of sense to me. Some follow-ups:

  • Should we update the merged and under review RFC PRs wit this rule?
  • It makes me think of that we don't really have rules to reject an RFC. Like what is the criteria to reject an RFC and should a rejected RFC PR directly be re-opened with improvements, or a new RFC PR has to be opened and associated with the rejected one. But this is not related to this PR, so we should discuss more in another thread.

@tqchen
Copy link
Member

tqchen commented Aug 4, 2021

Yes we can udpdate the merged RFC, the only one I am aware of is autoTIR RFC and perhaps @junrushao1994 can chime in

@tqchen
Copy link
Member

tqchen commented Aug 5, 2021

let us wait for another two days and if there is no objection, we can merge it in

@tqchen tqchen mentioned this pull request Aug 6, 2021
Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

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

This is a great improvement 😸

@tqchen tqchen merged commit 5320c63 into apache:main Aug 9, 2021
@tqchen
Copy link
Member

tqchen commented Aug 9, 2021

cc @junrushao1994 would be great if you can send a followup PR to rename the autoTIR RFC, and update links

@hogepodge
Copy link
Contributor

Thanks for the update. I'm glad to see that we're refining the RFC process as it goes into production.

@junrushao
Copy link
Member

Will apply the rule to meta schedule RFC this weekend

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.

6 participants