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

minor fix: removed < in changelog.md #12434

Closed
wants to merge 1 commit into from

Conversation

gautamkrishnar
Copy link
Contributor

Fixing a typo....
Fixes:#12430

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Apr 15, 2017
@gibfahn
Copy link
Member

gibfahn commented Apr 15, 2017

Change looks good but could you format your commit message according to the Contributing Guidelines?

Maybe something like this?

doc: fix typo in CHANGELOG.md

Fixes: https://github.com/nodejs/node/issues/12430

If you don't have time it can be rewritten by whoever lands this, but if you are going to help out more on the project (as I hope you do), then it saves work if you produce better commit messages.

CHANGELOG.md Outdated
@@ -29,7 +29,7 @@ release.
<tr>
<td valign="top">
<b><a href="doc/changelogs/CHANGELOG_V7.md#7.9.0">7.9.0</a></b><br/>
<a href="doc/changelogs/CHANGELOG_V7.md#7.8.0<">7.8.0<</a><br/>
<a href="doc/changelogs/CHANGELOG_V7.md#7.8.0<">7.8.0</a><br/>
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind fixing the extra < after the CHANGELOG_V7.md#7.8.0 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did it 👍

@mscdex
Copy link
Contributor

mscdex commented Apr 15, 2017

LGTM

@gautamkrishnar
Copy link
Contributor Author

gautamkrishnar commented Apr 16, 2017

@gibfahn Reworded and squashed 🔪
Thanks for informing me about the guidelines... Yes i would like to contribute my best ❤️

@TimothyGu
Copy link
Member

Landed in b837bd2.

@gautamkrishnar next time, make sure to include

Fixes: https://github.com/nodejs/node/issues/12430

in your commit message as well.

@TimothyGu TimothyGu closed this Apr 16, 2017
TimothyGu pushed a commit that referenced this pull request Apr 16, 2017
PR-URL: #12434
Fixes: #12430
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@gautamkrishnar gautamkrishnar deleted the patch-1 branch April 16, 2017 17:55
@gautamkrishnar
Copy link
Contributor Author

@TimothyGu thanks for correcting me. I will surely do that next time...

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.

7 participants