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

Add link to RFC Guide from jakobo/rfc #667

Merged
merged 4 commits into from
Apr 8, 2024
Merged

Add link to RFC Guide from jakobo/rfc #667

merged 4 commits into from
Apr 8, 2024

Conversation

spier
Copy link
Member

@spier spier commented Mar 23, 2024

Implements #666. Adding link to https://github.com/jakobo/rfc and description.

Details

  • Add link to repo jakobo/rfc, joining a guide for how to set up a company-internal RFC process.
  • Clarifying which 'Requests for Comments' we are referring to here

@spier spier added 2-structured Patterns with existing instances (Please see our contribution handbook for details) 📖 Type - Content Work Working on contents is the main focus of this issue / PR labels Mar 23, 2024
@spier spier changed the title Add link to RFC Guide jakobo/rfc Add link to RFC Guide from jakobo/rfc Mar 23, 2024
@spier
Copy link
Member Author

spier commented Mar 23, 2024

@tsadler1988 you might like this :)

@spier
Copy link
Member Author

spier commented Apr 8, 2024

@michael-basil as you were reading our patterns recently anyways, would you mind taking a look at this change here? It is a fairly small change, so I am just looking for a 2nd pair of eyes to see if the text is easy to understand and if this is an improvement to the pattern.

If you don't have time, no worries!

@michael-basil
Copy link

Taking a look now.

Copy link

@michael-basil michael-basil left a comment

Choose a reason for hiding this comment

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

@spier - After reading through the additional language and the attached links I believe it is an improvement to the pattern.

The links to the company-internal RFC process are opinionated helping to form a more concrete idea of how this could play out. There are some baked in biases and assumptions, but this is okay and serves the purpose well.

If it were me on the other side, with the level of understanding I currently have, I would approve this once the automated check failure is cleared.

@spier
Copy link
Member Author

spier commented Apr 8, 2024

@spier - After reading through the additional language and the attached links I believe it is an improvement to the pattern.

Great to hear! Thank you for the review!

The links to the company-internal RFC process are opinionated helping to form a more concrete idea of how this could play out. There are some baked in biases and assumptions, but this is okay and serves the purpose well.

If it were me on the other side, with the level of understanding I currently have, I would approve this once the automated check failure is cleared.

Thanks for reminding me of the issues with the vale check. I had mistakenly assumed that this was a false positive, however now I see that there is a systematic issues with our check, as it does not find out custom word lists at all. I am debugging this now.

This does not have to stop this PR from going live though. Will make fixes in a separate PR, once I figure out what's up there :)

Thanks again @michael-basil !

@spier spier merged commit 5172c35 into main Apr 8, 2024
10 checks passed
@spier spier deleted the add-rfc-guide branch April 8, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-structured Patterns with existing instances (Please see our contribution handbook for details) 📖 Type - Content Work Working on contents is the main focus of this issue / PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants