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 contributing guide. #2846

Closed
wants to merge 2 commits into from
Closed

Add contributing guide. #2846

wants to merge 2 commits into from

Conversation

SergioBenitez
Copy link
Member

@SergioBenitez SergioBenitez commented Aug 14, 2024

Adds a new CONTRIBUTING.md and modifies the README accordingly.

@the10thWiz curious to get your thoughts on this.

CONTRIBUTING rendered.
README rendered.

@SergioBenitez SergioBenitez force-pushed the contributing-guide branch 3 times, most recently from 856f8ed to 607602c Compare August 14, 2024 08:47
Copy link
Collaborator

@the10thWiz the10thWiz left a comment

Choose a reason for hiding this comment

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

Before reviewing, I wrote up a quick skeleton of what I thought should be in this, and the main thing you missed was what tests/examples should look like. Other than that, I think this is a great document.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
README.md Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
Comment on lines +86 to +88
To propose a new feature, create a [new issue] and select "feature request."
Follow the template, describing your solution in the _ideal solution_ or
_additional context_ sections as appropriate. Note that certain classes of
features require strong justification and exceedingly compelling reasons in
order to be taken into consideration. These include features that:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should reword this, and provide more detail on what we want from a feature request (most of this is about what not suggest).

Some things I would look for in a feature request:

  • Clear issue it's solving (e.g., A would now be possible, or A was very hard, but is now much easier)
  • Code examples for what someone using the new feature would look like (and the current work around if relevant)
  • Cohesive design
  • Limits potential for confusion and foot guns
  • As simple as possible (from the perspective of someone using Rocket)

Copy link
Member Author

Choose a reason for hiding this comment

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

The feature request issue template already has fields for most of these. Do you think it doesn't suffice?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's been a little while since I've opened an issue, so I had to go re-read it. Although I think it covers what we want, I do think there is some value of repeating it here.

Alternatively, we could shorten this section a bit, and simply refer to the issue creation for both what we are, and are not looking for.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@SergioBenitez
Copy link
Member Author

Reworked a bit of stuff based on your feedback. Let me know what you think.

@the10thWiz
Copy link
Collaborator

My main takeaway from the conversation, this time, is that we need a development guide at some point.

Other than that, I just left one comment open. After addressing that (and fixing CI :)), I think we are good to merge.

@SergioBenitez
Copy link
Member Author

The CI issues were unrelated and fixed on master. We definitely need a development guide at some point, but it's a big undertaking.

As far as being more specific about new features, what would you suggest, more specifically? Do you think we should improve the issue template and defer to it, add more wording here, or some combination thereof?

@SergioBenitez
Copy link
Member Author

Merged.

@SergioBenitez SergioBenitez added the pr: merged This pull request was merged manually. label Aug 18, 2024
@SergioBenitez SergioBenitez deleted the contributing-guide branch August 18, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: merged This pull request was merged manually.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants