Skip to content

DOC: Advise users to delete the PR template in message. - #152

Merged
jhlegarreta merged 1 commit into
InsightSoftwareConsortium:masterfrom
jhlegarreta:AdviceUsersToDeletePRTemplate
Nov 10, 2018
Merged

DOC: Advise users to delete the PR template in message.#152
jhlegarreta merged 1 commit into
InsightSoftwareConsortium:masterfrom
jhlegarreta:AdviceUsersToDeletePRTemplate

Conversation

@jhlegarreta

Copy link
Copy Markdown
Member

Improve the PR template message to highlight the fact that it is a set of
guidelines that should be removed for the final PR message.

@jhlegarreta

Copy link
Copy Markdown
Member Author

Folks, when the transition to GitHub was triggered many of the commit message contained only the template. I guess they were somehow automatically created.

Although the Insights, sec. Community of GitHub does encourage to have a PR template and I added it myself to ITK, I am not satisfied with the result: I believe it adds another action to the PR if we want to keep the message clean.

Until GitHub does a better job displaying the message outside of the commit message, this PR just adds a notice to users to remember to delete the template text.

Suggestions or modifications are welcome.

@thewtex thewtex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your attention to this @jhlegarreta .

Another thought: maybe we want to put the file in .github/PULL_REQUEST_TEMPLATE.md as described here:

https://blog.github.com/2016-02-17-issue-and-pull-request-templates/

?

Comment thread PULL_REQUEST_TEMPLATE.md Outdated
Comment thread PULL_REQUEST_TEMPLATE.md Outdated
@jhlegarreta
jhlegarreta force-pushed the AdviceUsersToDeletePRTemplate branch from 432f3ea to 88ef263 Compare November 9, 2018 03:01
@jhlegarreta

Copy link
Copy Markdown
Member Author

Will move them in a separate topic. Thanks @thewtex.

@jhlegarreta

Copy link
Copy Markdown
Member Author

The mentioned files are moved in #161.

@jhlegarreta jhlegarreta changed the title DOC: Advice users to delete the PR template in message. DOC: Advise users to delete the PR template in message. Nov 9, 2018

@thewtex thewtex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jhlegarreta !

The content-check is requesting a newline at the end of the file -- please go ahead with the merge after addressing this.

Improve the PR template message to highlight the fact that it is a set of
guidelines that should be removed for the final PR message.
@jhlegarreta
jhlegarreta force-pushed the AdviceUsersToDeletePRTemplate branch from 88ef263 to 16a81bb Compare November 10, 2018 01:12
@jhlegarreta

Copy link
Copy Markdown
Member Author

16a81bb adds the newline. Thanks for the heads-up Matt.

@hjmjohnson
hjmjohnson self-requested a review November 10, 2018 01:39
@jhlegarreta

Copy link
Copy Markdown
Member Author

The CircleCI build error is unrelated:
itkImageRandomNonRepeatingIteratorWithIndexTest (Failed)

Iterator in priority region test failed
[17, 19, 17] is outside the region (should be in)ImageRegion (0x7ffd1b8d1100)
  Dimension: 3
  Index: [0, 0, 0]
  Size: [50, 50, 50]

Merging.

@jhlegarreta
jhlegarreta merged commit 6f5bdab into InsightSoftwareConsortium:master Nov 10, 2018
@jhlegarreta
jhlegarreta deleted the AdviceUsersToDeletePRTemplate branch November 10, 2018 03:07
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…iceUsersToDeletePRTemplate

DOC: Advise users to delete the PR template in message.
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
…iceUsersToDeletePRTemplate

DOC: Advise users to delete the PR template in message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants