Add CONTRIBUTING.md and issue templates #1863
Conversation
There was a problem hiding this comment.
This looks great @emilyanndavis! I just noted a few small things we might consider adding.
| name: Bug report | ||
| about: Did you find a problem in InVEST? Let us know! | ||
| title: '' | ||
| labels: '' |
There was a problem hiding this comment.
Is it worth adding a default bug tag here?
There was a problem hiding this comment.
This crossed my mind, but it seemed like the consensus was to avoid adding labels on external bug reports until we've had a chance to review them. It could be useful to auto-label issues we create internally, but we had started moving toward using issue types instead of labels, so auto-labeling could lead to inconsistencies. (It doesn't seem to be possible to auto-assign an issue type from an issue template—or if there is, it's not documented.) I also thought we could add some new label, along the lines of needs-triage or pending, but without an existing process for handling such items, it seemed superfluous.
I'm inclined to try out the new templates without auto-attached labels, then configure appropriate labels if/when the need emerges. But I could be convinced if you feel strongly about it!
There was a problem hiding this comment.
Ah, that makes sense. I would probably also support adding some new label type for these issue templates, but happy to wait until we start seeing external contributions to discuss whether this is needed.
There was a problem hiding this comment.
I agree with adding some auto label that informs us it needs review from the maintainers. I also agree we can wait on this to see how it goes initially.
| name: Feature request | ||
| about: Have an idea for a new InVEST feature? Let us know! | ||
| title: '' | ||
| labels: '' |
There was a problem hiding this comment.
Should we automatically add an enhancement tag here?
| - `docs/34567-update-readme`. | ||
|
|
||
| ### Working on Code Changes | ||
| 1. Make sure you have followed the instructions in [Forking the Repository and Creating a Branch](#forking-the-repository-and-creating-a-branch). |
There was a problem hiding this comment.
Should we explicitly direct users to set up a development environment, e.g., with an editable invest installation? And/or point them to the invest readme for help with this?
There was a problem hiding this comment.
Good idea. I'll add a link to the readme.
Co-authored-by: Claire Simpson <112011324+claire-simpson@users.noreply.github.com>
…lyanndavis/invest into feature/1045-contributing-guidelines
| name: Bug report | ||
| about: Did you find a problem in InVEST? Let us know! | ||
| title: '' | ||
| labels: '' |
There was a problem hiding this comment.
Ah, that makes sense. I would probably also support adding some new label type for these issue templates, but happy to wait until we start seeing external contributions to discuss whether this is needed.
There was a problem hiding this comment.
This is great @emilyanndavis ! Just had a few comments and suggestions!
|
|
||
| ## To replicate | ||
| Provide detailed steps to replicate the behavior. | ||
| 1. |
There was a problem hiding this comment.
When I view this in rich diff, it shows 1. being on the previous line. Do we need a newline before the start of the list?
| Describe what actually happened. | ||
|
|
||
| ## Logfile(s) | ||
| If the problem occurred when running an InVEST model, please upload the logfile. (You can find this file in your workspace directory.) |
There was a problem hiding this comment.
Do we need the second sentence in parens?
| ## Logfile(s) | ||
| If the problem occurred when running an InVEST model, please upload the logfile. (You can find this file in your workspace directory.) | ||
|
|
||
| If you ran the model from the Workbench, please also upload the Workbench logfile. (To find this file, navigate to the Workbench's `About` menu, select `Report a problem`, then select `Find my logs`). |
There was a problem hiding this comment.
Similar, is putting the finding logfile details in parens necessary? To be fair, I'm no grammar wiz!
There was a problem hiding this comment.
I guess my thinking was, information about how to find a logfile is "extra" information that some people won't need, and parentheses are a simple visual cue to denote optional details. If someone already knows where to find their logfiles, they can skim the first few words in those parentheses ("You can find this file" or "To find this file") and feel confident they can skip everything else in that same set of parentheses.
|
|
||
| ## Reporting Bugs | ||
| Did you find a bug in the InVEST software? | ||
| 1. Search the [InVEST Open Issues on GitHub](https://github.com/natcap/invest/issues) to see if it has already been reported. |
There was a problem hiding this comment.
I'm wondering if we should also point people to do the Forums to search, to see if it's been discussed and any work arounds have been mentioned. Thoughts?
There was a problem hiding this comment.
We discussed this a lot while you were out, so yes—I do indeed have thoughts!
Initially, this section guided people to check both the forum and the GitHub issues, but then it got complicated. When it came to directing a user to post a topic to the forum or open a GitHub issue, we found it was difficult to define where the dividing line should be (between forum post and GitHub issue).
We didn't want to confuse people or discourage them from providing feedback out of fear of "doing it wrong", so we tried to simplify things as much as possible. That's how we ended up with an "Asking questions" section separate from the "Reporting bugs" section. Hopefully this will be enough to let people know what venues are available, and what they're meant for, while allowing them to use their own best judgment (and/or personal preference) in deciding which platform to gravitate towards.
I'm inclined to leave it as-is for now. We can always make changes later, if we find that the existing guidance isn't quite working the way we want it to!
CONTRIBUTING.md
Outdated
| 1. Search the [InVEST Open Issues on GitHub](https://github.com/natcap/invest/issues) to see if it has already been suggested. | ||
| 2. If you don’t find an existing issue, create a new issue. Select “Feature request”, and provide as much detail as you can. | ||
|
|
||
| Every proposed feature, large or small, will be weighed in terms of its potential impact on InVEST users and the capacity of the NatCap team to implement and maintain the feature. Please note that major features, including new InVEST models, must go through a peer-review process with our Platform Steering Committee. |
There was a problem hiding this comment.
We might want to mention that science related enhancements and issues, such as changes to model equations or how a model functions, will need to go through NatCap science review. We don't necessarily have the governance for this in place yet.
Maybe adding something like:
For science related changes such as the modification of equations and how a model functions, a NatCap science review will be needed to maintain the integrity of the science and implementation. We are currently still developing the governance for this type of review but encourage submissions.
There was a problem hiding this comment.
Sure. Do you think it's important to note that we don't have the governance just yet, or is that something we can address if/when these sorts of requests come in? It seems like the motivation to develop the governance might come from receiving a critical mass of requests, and I wonder if mentioning the lack of governance upfront might discourage people from submitting those requests (even though we say we encourage them).
There was a problem hiding this comment.
I went ahead and added this, altering the wording a a bit and leaving out the part about governance. Happy to update it if you feel strongly about any of the details!
…rects to the AWS-hosted URL) (natcap#1045)
Description
Resolves #1045.
Adds the following:
Checklist
- [ ] Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)- [ ] Updated the user's guide (if needed)- [ ] Tested the Workbench UI (if relevant)