-
Notifications
You must be signed in to change notification settings - Fork 178
PR template addition #411
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
PR template addition #411
Conversation
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.
One small nit, but not blocking. LGTM!
|
||
Fix problems which could leave deep sleep locked unintentionally, along with adding tests to verify this behavior is fixed. | ||
|
||
Tested locally with two targets, all toolchains. |
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 would recommend we be specific here, to encourage others to be specific as well.
Replace "two targets" with something like "K64F and K66F".
|
||
# Pull request type | ||
|
||
- [x] Fix |
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.
Like the checklist.
Copy edit file for grammar and complete sentences.
@adbridge @theotherjimmy @kjbracey-arm Please review |
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.
Agree with Cruz about being specific as to which two targets were tested against. But otherwise looks like a nice simple example. Can always be refined down line, good to get something in.
I had it first (named two targets I randomly chose) , but did not want to pick up only two targets (which ones we would select), lets see how this goes with users (what info are we going to get) and can add some info about being more specific |
Is it OK to merge this before 6078 merges? |
@AnotherButler Yup! It even seems like that PR is waiting on this! ARMmbed/mbed-os#6078 (comment) |
We would like to show users how a good pull request look like. I used code highlight for the template, is that fine or better solutions ?
The main points that should be coevred:
The template is being updated here ARMmbed/mbed-os#6078