First go at a PR template for tools-iuc #1991
Conversation
|
@Slugger70 cool :) |
|
Cool! Thanks @shiltemann. I'm also writing a comprehensive review checklist/guide for reviewers. But I reckon we should include something like @yhoogstrate's one here as well for reviewers too! |
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| * [ ] .shed.yml file ok | ||
| * [ ] Indentation is correct | ||
| * [ ] Tool version/build ok | ||
| * [ ] `<command/>`: file names are escaped |
There was a problem hiding this comment.
You're right. It makes more sense and that's what I put in the long form doc.
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| * [ ] `<command/>`: file names are escaped | ||
| * [ ] Tests | ||
| - [ ] Parameters are reasonably covered | ||
| - [ ] Test files are appropriate |
There was a problem hiding this comment.
Is there any rule (of thumb) for what are appropriate test files? Does the total need to be < few MB?
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| - [ ] Parameters are reasonably covered | ||
| - [ ] Test files are appropriate | ||
| * [ ] Toolshed user `tools-iuc` has access to associated toolshed repo(s) | ||
| * [ ] Complies with other best practice |
There was a problem hiding this comment.
.github/PULL_REQUEST_TEMPLATE.md
Outdated
|
|
||
| FOR REVIEWER: | ||
| * [ ] .shed.yml file ok | ||
| * [ ] Indentation is correct |
There was a problem hiding this comment.
Should the indentation always be 4 spaces?
|
I am not sure if it is worthwile adding it to the list, but maybe it's worth considering:
|
|
@yhoogstrate I think so.. Not sure about how to word it. @erasche what do you think? |
|
@Slugger70 yeah that's a good idea. We ask that in the cargo-port repo, really great contributors actually link the license as s requested, but most don't which annoys me. Maybe something like:
Edit: Otherwise this looks great! It's a lot of checkboxes and I expect many times contributors won't fill them all out, but it's a really nice reminder to have. |
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| * [ ] Tests | ||
| - [ ] Parameters are reasonably covered | ||
| - [ ] Test files are appropriate | ||
| * [ ] Toolshed user `tools-iuc` has access to associated toolshed repo(s) |
There was a problem hiding this comment.
the toolshed user is just iuc
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| @@ -0,0 +1,20 @@ | |||
| FOR CONTRIBUTOR: | |||
| * [ ] - I have read the [CONTRIBUTING.md](https://github.com/galaxyproject/tools-iuc/blob/master/CONTRIBUTING.md) document | |||
| * [ ] - This tool is appropriate for the tools-iuc repo (see [CONTRIBUTING.md](https://github.com/galaxyproject/tools-iuc/blob/master/CONTRIBUTING.md)) | |||
There was a problem hiding this comment.
Maybe merge this 2 in:
I have read the [CONTRIBUTING.md](https://github.com/galaxyproject/tools-iuc/blob/master/CONTRIBUTING.md) document and this tool is appropriate for the tools-iuc repo
| * [ ] Tool version/build ok | ||
| * [ ] `<command/>` | ||
| - [ ] Text parameters, input and output files `'single quoted'` | ||
| - [ ] Use of `<![CDATA[ ... ]]>` tags |
There was a problem hiding this comment.
<![CDATA[ ... ]]> is recommended also for the <help/> section.
There was a problem hiding this comment.
maybe just add a checklist item that the help text is valid restructured text? (and uses CDATA tags)
There was a problem hiding this comment.
Ok. I'm also (in parallel) writing a comprehensive guide for reviewers. Should this just be a short summary of that or be a stand alone thing?
There was a problem hiding this comment.
@Slugger70 I was wondering if we should merge this list in the one you are adding in #2000 , and replace it here with a link to the guide for reviewers?
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| * [ ] Tests | ||
| - [ ] Parameters are reasonably covered | ||
| - [ ] Test files are appropriate | ||
| * [ ] Toolshed user `iuc` has access to associated toolshed repo(s) |
There was a problem hiding this comment.
This can go as sub-point of .shed.yml file ok.
|
👍 |
From the Tools IUC meeting at GCC this year, I was tasked with working on a review checklist. I thought that having a PR checklist template similar to the bioconda-recipes repo would be useful here as well. This is a WIP and please comment!