-
Notifications
You must be signed in to change notification settings - Fork 222
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 template for wrapping new module #1629
Conversation
Co-authored-by: Wei Ji <[email protected]>
- [ ] Write detailed docstrings for all functions/methods. | ||
- [ ] If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs. | ||
- [ ] If adding new functionality, add an example to docstrings or tutorials. |
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 feel like these checklist items could be consolidated and/or reworded to be a bit better. Any suggestions?
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. Perhaps the following two could be consolidated to "Try to get all checks passing, including style checks and test coverage."
- Run make format and make check to make sure the code follows the style guide.
- Add tests for new features or tests that would have caught the bug that you're fixing.
For the following four, if you want I could add a section to 'Contributing code' for adding new functions/methods/classes (including GMT wrappers) that covers this information such that the checklist is simply 'Follow guide in <link to contributing guide section>
if adding a new function/method/class'.
- Add new public functions/methods/classes to doc/api/index.rst.
- Write detailed docstrings for all functions/methods.
- If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
- If adding new functionality, add an example to docstrings or tutorials.
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.
@weiji14, since this PR has a few approvals already would you mind if I merged this and worked on your suggestions in a separate PR? That way the other reviewers would not need to re-look at existing work. I have the updates partly done, but realistically won't have time to finish it until at least late next week due to AGU rush, GMT tasks, and planned leave.
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.
Ok, sounds good to me to merge this new issue template first. The Pull Request template does need a bit of a refresh so that can be done in a follow up Pull Request (but please do prioritize AGU first).
- [ ] Write detailed docstrings for all functions/methods. | ||
- [ ] If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs. | ||
- [ ] If adding new functionality, add an example to docstrings or tutorials. |
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.
Ok, sounds good to me to merge this new issue template first. The Pull Request template does need a bit of a refresh so that can be done in a follow up Pull Request (but please do prioritize AGU first).
* Add issue template for wrapping new module Co-authored-by: Wei Ji <[email protected]>
Description of proposed changes
This PR adds an issue template for wrapping a new module, to help track the implementation over several PRs.
Part of #1599
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version