Skip to content

Commit

Permalink
doc: missing brackets
Browse files Browse the repository at this point in the history
PR-URL: #32657
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
  • Loading branch information
sla100 authored and MylesBorins committed Apr 17, 2020
1 parent 6bcf968 commit 5e807c1
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ console.log('I just set some globals!');
const { createRequire } = getBuiltin('module');
const require = createRequire(process.cwd + '/<preload>');
const require = createRequire(process.cwd() + '/<preload>');
// [...]
`;
}
Expand Down

3 comments on commit 5e807c1

@Trott
Copy link
Member

@Trott Trott commented on 5e807c1 Apr 17, 2020

Choose a reason for hiding this comment

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

I would ask that we (on the project) edit the commit message when landing to make them more useful. Given the small nature of the change, this one is acceptable but it would be better as something like this: doc: add missing brackets to sample code in esm.md

(Also, pull-requests.md says the first word needs to be an imperative verb, so please let's enforce that too or else change our docs.)

@MylesBorins
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry about that @Trott

Is there a way that we could lint for the imperative verb with git node land? I was unaware of that particular rule and saw green in git node land

Obviously could have done a better job on the message as a whole (already had added subsystem when landing)

@Trott
Copy link
Member

@Trott Trott commented on 5e807c1 Apr 17, 2020

Choose a reason for hiding this comment

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

Is there a way that we could lint for the imperative verb with git node land?

Not reliably, and I think people will be more annoyed by that being unreliable than I will be by the commit message here and there that doesn't match it. Honestly, I've wondered if we should remove that requirement, or at least turn it into a recommendation. I understand why it's there, but we just aren't that rigorous with our changelog messages, so it might be pointless.

Please sign in to comment.