-
Notifications
You must be signed in to change notification settings - Fork 53
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 issue and PR templates #2097
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this!
<!-- * file.rs, `add_integers` function --> | ||
|
||
### How to test this PR: | ||
<!-- * E.g. `just async_std test` --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we expecting the reviewer to run the tests? I think we could skip this "How to test" section if no manual testing is needed and we could just use the CI results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair! Perhaps I'll leave it as an optional field.
|
||
<!-- Complete the following items before creating this PR | ||
* Are the proper people tagged to review it? | ||
* Have you listed yourself as the assignee? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this is necessary since the PR author is usually the same as the assignee. I think the author is also a filter attribute if we want to search for and filter PRs on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was something @dailinsubjam brought up. I am fine either way. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could help but not necessary, assignee's avatar will appear right after PR on the searching board which might be easier to find, but looks like we can also filter PR by selecting the author.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I thought it made sense when it was brought up in the meeting yesterday! But on second thought, it's probably not necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll remove it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dailinsubjam and @shenkeyao here is the new PR: #2103
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved there. Thanks Ellie!
No description provided.