Uh oh!
There was an error while loading. Please reload this page.
Check issue template - #20
Conversation
Thanks so much for opening your first PR here! |
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update some of our documentation based on your changes. |
| function checkTemplateMatch(body) { | ||
| const lines = issueTemplate.split("\n*") | ||
| lines.forEach(line => { | ||
| if (!body.includes(line)) { |
There was a problem hiding this comment.
How does this work for templates with forms you have to fill out? For example:
**Foo version**: [enter your version of Foo here]**OS version**: [enter your operating system’s name and version here]There was a problem hiding this comment.
Right now this only checks that you correctly include all of the lines, as if you were filling out a form, but I'm not sure that's the best approach. Some times issue templates follow a branching style, like if it's a bug fill out area A, a feature request area B, etc.
As I said before, I think a solid approach for step one would be checking that the issue body isn't exactly the issue template ( ie !body.includes(issueTemplate))
There was a problem hiding this comment.
so this is what i thought about earlier, and our issues in electron typically stem from people deleting things, so i think i want to incorporate more granular checking at some point. For now though i agree, and i can iterate from there!
hiimbex
left a comment
There was a problem hiding this comment.
this is on the right track, I'm still not sure about the best algorithmic approach to solving this though 🤔
| comment = defaultComment | ||
| } | ||
| // set comment to default if none passed it | ||
| const eventComment = (!comment) ? defaultComment : comment |
There was a problem hiding this comment.
for future reference, I'd prefer style changes like these (basically all of lib/getComment.js) in a separate PR.
There was a problem hiding this comment.
reverted, open in separate PR :)
| "id": 35471 | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
this fixture is identical to test/events/issueSuccessEvent.json, but I think ideally eventually the test will look more like an offshoot of a specific issue template
There was a problem hiding this comment.
yeah oops i know sorry i have yet to change it for issue template purposes
| } | ||
| } | ||
| if (config.matchIssueTemplate) { |
There was a problem hiding this comment.
👍 to this config name, make sure to add it to the readme
| function checkTemplateMatch(body) { | ||
| const lines = issueTemplate.split("\n*") | ||
| lines.forEach(line => { | ||
| if (!body.includes(line)) { |
There was a problem hiding this comment.
Right now this only checks that you correctly include all of the lines, as if you were filling out a form, but I'm not sure that's the best approach. Some times issue templates follow a branching style, like if it's a bug fill out area A, a feature request area B, etc.
As I said before, I think a solid approach for step one would be checking that the issue body isn't exactly the issue template ( ie !body.includes(issueTemplate))
| @@ -0,0 +1,17 @@ | |||
| const res = await context.github.repos.getContent(this.repo({ | |||
| path: '.github/ISSUE_TEMPLATE.md' | |||
| })) | |||
There was a problem hiding this comment.
also I think CI is failing due to this needing to be inside something async
| # *OPTIONAL* Label to be added to Issues and Pull Requests with insufficient information given | ||
| requestInfoLabelToAdd: needs-more-info | ||
| # *OPTIONAL* Require the issue to match an issue template you have in your repository. |
There was a problem hiding this comment.
Do you think maybe this could be better rephrased to explain what it's actually checking?
hiimbex
commented
Oct 21, 2017
One last nit comment and then this should be good to go! |
hiimbex
commented
Oct 21, 2017
Actually, any chance you wanna add a test for this? |
hiimbex
commented
Apr 16, 2018
hey @codebytere no rush, but any interest in finishing this up? |
codebytere
commented
Apr 16, 2018
@hiimbex would https://github.com/behaviorbot/request-info/pull/32/files not take its place? |
hiimbex
commented
Apr 16, 2018
well that's for pull request templates, not issue templates, so I think this code would still be useful |
shiftkey
commented
Oct 18, 2018
I think this should be covered by #39 which was just merged. @codebytere let me know if there's anything more do to with that. |
Todo:
/cc @hiimbex