-
Notifications
You must be signed in to change notification settings - Fork 25
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
Create pull_request_template.md #138
Conversation
I really like this, what do you think @nickytonline? |
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.
@@ -0,0 +1,39 @@ | |||
## Status |
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.
A status of the PR makes sense, but there are already mechanisms in place to handle this. If the pull request is still in development, it can be created/set to a Draft Pull Request.
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 agree on that, and we should probably include that in the to be made contribution.md as Draft Pull Requests is quite new and not everyone know about that yet.
## Description | ||
A few sentences describing the overall goals of the pull request's commits. | ||
|
||
## Related PRs |
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.
Related PRs could be changed to related tickets and documents so that it's more broad as to what's related.
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 makes sense as often we have to refer to the support tickets and documentation of the different providers etc.
- [ ] Documentation | ||
|
||
|
||
## Deploy Notes |
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.
Deploy notes, if any, makes sense for an application, but in our case, once the code is merged, it will get versioned and deployed via npm publish
. There are the demo websites, but I imagine they automatically deploy to Netlify once merged to the main branch @raae?
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.
Yes, that is correct (or at least very soon will be) @nickytonline. So I agree.
Thank you very much. Should I make a new PR with these changes? or anything else? |
I'll let @raee chime in and then we can go from there. |
I agree with @nickytonline, se inline comments. There is no need for a new Pull Request, if you do the changes and push to your branch it should be reflected here. Thank you again, so happy more people are joining in. |
Would you like to keep working on this @gunesnt? |
Hi @gunesnt! Just following up one more time to see if you'd like to see this through. If not, no worries. If, not, it'll be a great starting point for another contributor. |
Adds a pull request template to the project.