-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[#897] - Add issue templates #1003
Conversation
…t request, feature request, and questions.
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.
Im inclined to approve this.
However, since I don't really have any expertise in setting these up,
can somebody else take a look as well :)
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.
Just a little comment about the version list in the bug report template, otherwise LGTM.
@EdgarRamirezFuentes Thank you for your contribution!
attributes: | ||
label: Shiro version | ||
description: Describe the Shiro version. | ||
options: |
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'm not sure this is a good idea to provide a version list because we will need to maintain it up to date.
May be a simple input text field should be enough.
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.
Understood!
I made the changes in the bug report file, and switched the Shiro version to a textarea. 😄
In the bug_report.yml file switched the dropdow to specify the version of Shiro to a textarea.
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.
LGTM!
@bmarwell is it ok with what you had in mind? |
|
||
- type: dropdown | ||
attributes: | ||
label: Environment |
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 we can omit the environment b/c Java.
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.
Maybe replace with a text field for application server, Java version, etc
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.e Environment? :)
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.
|
||
- type: textarea | ||
attributes: | ||
label: What happened |
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 was the actual outcome"
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.
It's missing "what did you expect instead"
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.
So, I change the label to "what was the actual outcome" instead of "What happened", and add a new textarea with the label "what did you expect instead"?
|
||
- type: checkboxes | ||
attributes: | ||
label: Are you willing to submit PR? |
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.
This is not required imho. Or maybe rephrase it? @bdemers might want to suggest something?
It's a great start! |
I would say to merge this "as is" and then update it when we figure out what the exact copy should read. |
@lprimak |
@bmarwell What do you think? |
Yes, please change accordingly. |
Done! |
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.
Yup, that's a start!
@EdgarRamirezFuentes thanks for your first contribution! |
Resolve #897 by adding bug report, documentation related, enhancement request, feature request, and question issue templates.
Following this checklist to help us incorporate your contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a GitHub issue. Your pull request should address just this issue, without pulling in other changes.
[#XXX] - Fixes bug in SessionManager
,where you replace
#XXX
with the appropriate GitHub issue. Best practiceis to use the GitHub issue title in the pull request title and in the first line of the commit message.
fixes #XXX
if merging the PR should close a related issue.mvn clean install apache-rat:check
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.git rebase -i
.Trivial changes like typos do not require a GitHub issue (javadoc, comments...).
In this case, just format the pull request title like
[DOC] - Add javadoc in SessionManager
.If this is your first contribution, you have to read the Contribution Guidelines
If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement
if you are unsure please ask on the developers list.
To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.