Skip to content

Conversation

@bf4
Copy link
Contributor

@bf4bf4 commented Mar 6, 2017

Follows #11189

To be considered a breaking change per #11189 (comment)

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 6, 2017
@jasnell
Copy link
Member

Can you format the first line of the commit message in accordance with the guidelines in the contribution guide?

@addaleaxaddaleax added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 6, 2017
@mscdexmscdex added semver-major PRs that contain breaking changes and should be released in the next major version. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Mar 7, 2017
@bf4
Copy link
ContributorAuthor

bf4 commented Mar 7, 2017

@jasnell

Can you format the first line of the commit message in accordance with the guidelines in the contribution guide?

I find the contribution guidelines kind of hard to parse.

The first line should be 50 characters or less and contain a short description of the change. All words in the description should be in lowercase with the exception of proper nouns, acronyms, and the ones that refer to code, like function/variable names. The description should be prefixed with the name of the changed subsystem and start with an imperative verb. Example: "net: add localAddress and localPort to Socket"

Are you asking me to format the commit message in lowercase? The PR description in lowercase? Or what subsystem would you call this? src: correct typos

If your patch fixes an open issue, you can add a reference to it at the end of the log. Use the Fixes: prefix and the full issue URL. For other references use Refs:. For example:

Should I add to the body of the commit message Refs: https://github.com/nodejs/node/pull/11189#discussion_r99509433

@lpinca
Copy link
Member

Are you asking me to format the commit message in lowercase?

Yes. Something like src: fix typos in node_lttng_provider.h should work.
You can detail what typos have been fixed in the commit body but I think it's not necessary.
I would also not include the Refs: metadata as that comment does not provide any additional useful information imo, it doesn't harm though.

@bf4bf4force-pushed the correct_typos_breaking_change branch from 6617cb0 to cfe7339CompareMarch 7, 2017 15:40
@bf4bf4 changed the title Breaking change, typo "s/Unkown/Unknown GC Type"src: fix typos in node_lttng_provider.hMar 7, 2017
@bf4
Copy link
ContributorAuthor

bf4 commented Mar 7, 2017

@jasnell@lpinca Done

@jasnell
Copy link
Member

Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

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

@addaleax
Copy link
Member

Landed in 1125c8a, thanks for the PR!

addaleax pushed a commit that referenced this pull request Mar 10, 2017
Is a semver-breaking change Refs: #11189 (comment) PR-URL: #11723 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@bf4bf4 deleted the correct_typos_breaking_change branch March 13, 2017 01:25
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
Is a semver-breaking change Refs: nodejs#11189 (comment) PR-URL: nodejs#11723 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@jasnelljasnell mentioned this pull request Apr 4, 2017
gibfahn pushed a commit that referenced this pull request Apr 22, 2017
- Can now link to 'Commit Guidelines' from pull requests - Breaks up commit requirements and recommendations PR-URL: #11732 Refs: #11723 (comment) Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
evanlucas pushed a commit that referenced this pull request Apr 25, 2017
- Can now link to 'Commit Guidelines' from pull requests - Breaks up commit requirements and recommendations PR-URL: #11732 Refs: #11723 (comment) Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
evanlucas pushed a commit that referenced this pull request May 1, 2017
- Can now link to 'Commit Guidelines' from pull requests - Breaks up commit requirements and recommendations PR-URL: #11732 Refs: #11723 (comment) Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
evanlucas pushed a commit that referenced this pull request May 2, 2017
- Can now link to 'Commit Guidelines' from pull requests - Breaks up commit requirements and recommendations PR-URL: #11732 Refs: #11723 (comment) Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
gibfahn pushed a commit that referenced this pull request May 16, 2017
- Can now link to 'Commit Guidelines' from pull requests - Breaks up commit requirements and recommendations PR-URL: #11732 Refs: #11723 (comment) Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.semver-majorPRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@bf4@jasnell@lpinca@addaleax@cjihrig@mscdex@nodejs-github-bot