Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Update PR Template #9919

Merged
merged 6 commits into from Mar 17, 2018
Merged

Conversation

eric-haibin-lin
Copy link
Member

Removed code style checking from the template. It is checked by the CI anyway. No PR can be merged without passing this.

@marcoabreu
Copy link
Contributor

Could we add checkboxes to all items in the list?

- Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
would become
- [ ] Unit tests are added for small changes to verify correctness (e.g. adding a new operator)

@szha
Copy link
Member

szha commented Feb 28, 2018

I think the list items are intended as enumeration of possible situations. They usually don't apply all at once.

@eric-haibin-lin
Copy link
Member Author

+1 on sheng's comment

@lebeg
Copy link
Contributor

lebeg commented Mar 1, 2018

Could we add a note that not applicable items should be removed from the description? It's sometimes hard to see the actual message between the default text lines

@eric-haibin-lin
Copy link
Member Author

Done

@szha
Copy link
Member

szha commented Mar 5, 2018

@eric-haibin-lin would you add an item to remind the committers to check the API doc of the PR? See details in #9928. It should read like:
Committers should check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html. (e.g. link)

@rahul003
Copy link
Member

rahul003 commented Mar 9, 2018

@szha That is nice! Didn't know we were building API docs for each PR.
+1 for adding that

@eric-haibin-lin
Copy link
Member Author

Done

@marcoabreu
Copy link
Contributor

Could we also add an item to remind people linking the JIRA issue?

@eric-haibin-lin
Copy link
Member Author

Done. Although I'm a little bit worried that all these reminders make the template kind of heavy

@marcoabreu marcoabreu merged commit 50c694d into apache:master Mar 17, 2018
@marcoabreu
Copy link
Contributor

Agree, we might have to do a proper cleanup of that template and make it more lightweight.

@eric-haibin-lin eric-haibin-lin deleted the eric-haibin-lin-patch-2 branch March 27, 2018 04:03
ashokei pushed a commit to ashokei/incubator-mxnet that referenced this pull request Mar 27, 2018
* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md
jinhuang415 pushed a commit to jinhuang415/incubator-mxnet that referenced this pull request Mar 30, 2018
* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md
rahul003 pushed a commit to rahul003/mxnet that referenced this pull request Jun 4, 2018
* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md
zheng-da pushed a commit to zheng-da/incubator-mxnet that referenced this pull request Jun 28, 2018
* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md

* Update PULL_REQUEST_TEMPLATE.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants