Skip to content
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

github: put description of PR to the end of tmpl #5378

Conversation

indutny
Copy link
Member

@indutny indutny commented Feb 23, 2016

Pull Request check-list

Please make sure to review and check all of these items:

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done afterwards /
while the PR is open.

Affected core subsystem(s)

Please provide affected core subsystem(s) (like buffer, cluster, crypto, etc)

Description of change

It looks like github appends commit log right after the template
contents. Seeing this it does not look logical to ask for details first.

This commit puts Description of change part to the end of the
template.

NOTE: I had to manually re-order this to put Description of change to the bottom in this PR

It looks like github appends commit log right after the template
contents. Seeing this it does not look logical to ask for details first.

This commit puts `Description of change` part to the end of the
template.
@indutny
Copy link
Member Author

indutny commented Feb 23, 2016

cc @nodejs/collaborators @Fishrock123 @mscdex and all others.

@indutny indutny added the meta Issues and PRs related to the general management of the project. label Feb 23, 2016
@mscdex
Copy link
Contributor

mscdex commented Feb 23, 2016

Now that I can see it rendered, can this line be moved to the end of the previous line?

@indutny
Copy link
Member Author

indutny commented Feb 23, 2016

@mscdex it will go over 80 columns, perhaps I should move afterwards / to the next line instead?

@indutny
Copy link
Member Author

indutny commented Feb 23, 2016

@mscdex hopefully, ack

@Fishrock123
Copy link
Contributor

LGTM. Perhaps in a separate PR I'd like to not that checklist items that do not apply can be removed. (i.e. doc changes, example: #5391)

@indutny
Copy link
Member Author

indutny commented Feb 24, 2016

Landed in 4bd8b20, thank you! Please feel free to open follow-up PR.

@indutny indutny closed this Feb 24, 2016
@indutny indutny deleted the feature/follow-up-to-81e35b5a2956604a6a3b9bb795a74a342702cc12 branch February 24, 2016 00:21
indutny added a commit that referenced this pull request Feb 24, 2016
It looks like github appends commit log right after the template
contents. Seeing this it does not look logical to ask for details first.

This commit puts `Description of change` part to the end of the
template.

PR-URL: #5378
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this pull request Feb 27, 2016
It looks like github appends commit log right after the template
contents. Seeing this it does not look logical to ask for details first.

This commit puts `Description of change` part to the end of the
template.

PR-URL: #5378
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants