-
Notifications
You must be signed in to change notification settings - Fork 609
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
Contributing.md link & PR template simplifications & Added ISSUE_TEMPLATE.md #1399
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1399 +/- ##
==========================================
- Coverage 19.82% 19.53% -0.30%
==========================================
Files 202 226 +24
Lines 27685 30950 +3265
==========================================
+ Hits 5489 6045 +556
- Misses 21175 23800 +2625
- Partials 1021 1105 +84
Continue to review full report at Codecov.
|
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.
Nice work. I like the issue template. Left a couple of comments too, let me know what you think
.github/pull_request_template.md
Outdated
## Documentation and Release Note | ||
|
||
- Does this pull request introduce a new feature or user-facing behavior changes? (yes / no) | ||
- Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? (yes / no) | ||
- How is the feature or change documented? (not applicable / specification (`x/<module>/spec/`) / [Osmosis docs repo](https://github.com/osmosis-labs/docs) / not documented) | ||
|
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 this one was better, it forced acknowledgement of the options for docs updates
.github/pull_request_template.md
Outdated
|
||
*(Please pick one of the following options)* | ||
|
||
This change is a trivial rework / code cleanup without any test coverage. | ||
|
||
*(or)* | ||
|
||
This change is already covered by existing tests, such as *(please describe tests)*. | ||
|
||
*(or)* | ||
|
||
This change added tests and can be verified as follows: | ||
|
||
*(example:)* |
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.
Do others dislike this? I thought this was great, I just choose the one I want and delete everything else
.github/pull_request_template.md
Outdated
|
||
## Brief change log | ||
|
||
## Brief change log (include any info relevant to tests)) |
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.
what is (include any info relevant to tests)
for?
Hmm, got it, depending on additional input I might just revert most if not
all of the quick changes in the pr template…
…On Tue, May 3, 2022 at 3:43 PM Dev Ojha ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .github/pull_request_template.md
<#1399 (comment)>:
> -
-*(Please pick one of the following options)*
-
-This change is a trivial rework / code cleanup without any test coverage.
-
-*(or)*
-
-This change is already covered by existing tests, such as *(please describe tests)*.
-
-*(or)*
-
-This change added tests and can be verified as follows:
-
-*(example:)*
Do others dislike this? I thought this was great, I just choose the one I
want and delete everything else
—
Reply to this email directly, view it on GitHub
<#1399 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASH4FPXBUCYMEK3JGMG77A3VIF6UZANCNFSM5U7VECAQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Alright folks! Reverted changes to pull_request_template.md but issue_template.md mostly remains the same. Also fixed @alexanderbez 's comment on README.md. Let me know what you all think :) |
.github/issue_template.md
Outdated
@@ -0,0 +1,17 @@ | |||
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ |
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.
Shouldn't this exist under .github/ISSUE_TEMPLATE
? Also, what kind of template is this? For bugs? I propose something like the following: https://github.com/umee-network/umee/blob/main/.github/ISSUE_TEMPLATE/bug-report.md
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 didn't happen to know we had a .github/ISSUE_TEMPLATE already if we did (I don't happen to see it), I can definite create a /ISSUE_TEMPLATE folder though. And this was originally supposed to be for generic issues, not specifically bugs.
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.
Yeah, we should have templates per type IMO (i.e. bugs, general proposal, features, etc...). Otherwise, having a template isn't all that useful.
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.
Alright, I'll make that then
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 think general proposals / features are done in a separate repo, and questions are done in our discord server, so I currently only implemented the bug reporter template, the generic issue template along with the option to still open blank issues. Let me know your thoughts :)
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 think bug report and general template is fine. I don't really understand why features are done in a separate repo, but I don't wanna bike shed here.
@@ -0,0 +1 @@ | |||
blank_issues_enabled: true |
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.
nit: missing new line at the end
Just wondering, has this been merged in yet? |
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Closes: #1263 (Not merging until this also gets closed on docs side)
What is the purpose of the change
Brief change log
Tests / Documentation