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.

Sign up for GitHub

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

Jump to bottom

RFC: Label RFCs With Spec #108

Merged
merged 5 commits into from
Sep 9, 2020
Merged

RFC: Label RFCs With Spec #108

merged 5 commits into from
Sep 9, 2020

Conversation

ForestEckhardt
Copy link
Contributor

@ForestEckhardt ForestEckhardt commented Aug 14, 2020 •
edited
Loading

@ForestEckhardt ForestEckhardt requested a review from a team as a code owner August 14, 2020 18:38
@ForestEckhardt
RFC to label RFCs
161fadb 
Signed-off-by: Forest Eckhardt <feckhardt@pivotal.io>
@kvedurmu
Copy link
Contributor

This would be a great change! With the number of RFCs flowing in here, it's hard to keep track of what's applicable to my team. It would be even better if accepted RFCs could be grouped accordingly as well so users can easily go back and see all relevant RFCs for the Buildpack Interface, Platform, etc.

@nebhale nebhale requested a review from a team August 14, 2020 19:48
@nebhale
Copy link
Contributor

nebhale commented Aug 14, 2020

Well, @kvedurmu you've got a spec change that needs some attention 😜

@yaelharel
Copy link
Contributor

This is a great idea! It's really hard (especially for new contributors) to track all of the RFCs. Applying labels will help a lot!
I hope we'll start do it soon!

ekcasey
ekcasey approved these changes Aug 17, 2020
View reviewed changes
Copy link
Member

@ekcasey ekcasey left a comment

Choose a reason for hiding this comment

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

I like this idea. Once upon a time we tried labeling we subteam names, which are sorta-kinda-similarish, but gave up. I think labeling with the API is more useful to a broader audience.

text/0000-tag-rfcs.md Outdated Show resolved Hide resolved
text/0000-tag-rfcs.md Outdated Show resolved Hide resolved
text/0000-tag-rfcs.md Outdated Show resolved Hide resolved
@ForestEckhardt @ekcasey
Apply suggestions from code review
35e58b1 
Cleans up meta

Co-authored-by: Emily Casey <emilykimballcasey@gmail.com>
Signed-off-by: Forest Eckhardt <feckhardt@pivotal.io>
@hone
Copy link
Member

hone commented Aug 19, 2020

I like this idea a lot and something that has come up when talking to our languages team at Heroku to make it easier to know what they should be reviewing. I believe there's some more detail that is missing. Who is responsible of labeling? Is that something we just do during the weekly WG on Wed? Also as an alternative or an addition, is it useful to label by "target audience":

  • Buildpack Author
  • CNB Contributor
  • Platform Operator

@nebhale nebhale requested a review from a team August 19, 2020 19:09
@ForestEckhardt
Adds target audience as tag as well as adding a process for tagging RFCs
c5968a6 
- Change RFC file name to better reflect what it is actually purposing

Signed-off-by: Forest Eckhardt <feckhardt@pivotal.io>
@ForestEckhardt
Copy link
Contributor Author

@hone I love the idea of adding the target audience in addition to API labeling. Let me write up something quick to include this as well as a process around labeling the RFCs.

hone
hone reviewed Aug 26, 2020
View reviewed changes
text/0000-label-rfcs.md Outdated
# Implementation
[Implementation]: #implementation

During any Working Group meeting, when a new RFC is brought up it should at
Copy link
Member

Choose a reason for hiding this comment

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

Should we try to do this during the weekly review of RFCs or would that bog that process down too much? Thoughts @sclevine?

Copy link
Member

Choose a reason for hiding this comment

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

From the WG: we should be able to handle this as we go through the RFCs every week if you want to update @ForestEckhardt

Copy link
Contributor Author

@ForestEckhardt ForestEckhardt Aug 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@hone Sounds good to me. I will update this when I get the chance.

@nebhale nebhale requested a review from a team August 26, 2020 19:08
@nebhale nebhale requested a review from a team August 26, 2020 19:09
@ForestEckhardt
RFCs will be labeled during weekly RFC review
83dbff5 
Signed-off-by: Forest Eckhardt <feckhardt@pivotal.io>
@nebhale
Copy link
Contributor

nebhale commented Aug 31, 2020

Final Comment Period with merge disposition, closing on 7 September, 2020

dfreilich
dfreilich approved these changes Sep 3, 2020
View reviewed changes
text/0000-label-rfcs.md Show resolved Hide resolved
@ForestEckhardt ForestEckhardt force-pushed the rfc-tags branch 2 times, most recently from e21b64b to bce2ac9 Compare September 3, 2020 14:14
@ForestEckhardt @dfreilich
Update text/0000-label-rfcs.md
c3f1e39 
Co-authored-by: David Freilich <dfreilich@vmware.com>
Signed-off-by: Forest Eckhardt <feckhardt@pivotal.io>
nebhale added a commit that referenced this pull request Sep 9, 2020
@nebhale
RFC 0059
1be06d5 
[#108]

Signed-off-by: Ben Hale <bhale@vmware.com>
@nebhale nebhale merged commit f9d9919 into buildpacks:main Sep 9, 2020
@ekcasey ekcasey mentioned this pull request Oct 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hone hone hone approved these changes

@yaelharel yaelharel yaelharel approved these changes

@dfreilich dfreilich dfreilich approved these changes

@ekcasey ekcasey ekcasey approved these changes

@nebhale nebhale nebhale approved these changes

@jkutner jkutner jkutner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ForestEckhardt @kvedurmu @nebhale @yaelharel @hone @jkutner @ekcasey @dfreilich