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.

Sign up for GitHub

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

Jump to bottom

doc typofix: fetaures -> features #3522

Closed
wants to merge 1 commit into from
Closed

doc typofix: fetaures -> features #3522

wants to merge 1 commit into from

Conversation

mLuby
Copy link

@mLuby mLuby commented Oct 26, 2015

Noticed it when using node --v8-options | grep harmony.

@jbergstroem jbergstroem added the doc Issues and PRs related to the documentations. label Oct 26, 2015
@jbergstroem
Copy link
Member

Needs some minor commit message massage (suggested: doc: fix typo (fetaures -> features)) otherwise LGTM.

@jbergstroem
Copy link
Member

@mLuby you probably want to update your author info as well (you edited through github, no?).

thefourtheye
thefourtheye reviewed Oct 26, 2015
View reviewed changes
deps/v8/src/flag-definitions.h
@@ -178,7 +178,7 @@ DEFINE_BOOL(strong_this, true, "don't allow 'this' to escape from constructors")

DEFINE_BOOL(es_staging, false, "enable all completed harmony features")
DEFINE_BOOL(harmony, false, "enable all completed harmony features")
DEFINE_BOOL(harmony_shipping, true, "enable all shipped harmony fetaures")
DEFINE_BOOL(harmony_shipping, true, "enable all shipped harmony features")
Copy link
Contributor

Choose a reason for hiding this comment

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

This comes from upstream. We may not be able to edit them directly like this, unless there is a compelling reason.

@mLuby
doc typofix: fetaures -> features
ffbf5d2 
simple typo found in harmony mode documentation:
fetaures should be features
@jbergstroem
Copy link
Member

Oh, didn't consider that. Upstream fix then.

@mLuby
Copy link
Author

mLuby commented Oct 26, 2015

A search of the entire repo only found those two instances of the typo. Wonder where the upstream mistake is—some external source perhaps?

@jbergstroem
Copy link
Member

@mLuby this'd be it: https://github.com/v8/v8/blob/b3c719ebbad6c87afefa33a7d0b3f412b2e304db/src/flag-definitions.h#L184

The procedure for sending patches to v8 is a bit of a hurdle.

@mscdex
Copy link
Contributor

mscdex commented Oct 26, 2015

+1 for fixing this upstream in v8. The less floated patches the better.

@mscdex mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Oct 26, 2015
@thefourtheye
Copy link
Contributor

Submitted a patch https://codereview.chromium.org/1413023003/

@thefourtheye
Copy link
Contributor

Closing this PR, as the patch landed in upstream. @mLuby Thanks for trying to fix this :-)

@mLuby
Copy link
Author

mLuby commented Oct 26, 2015

Thank you for actually fixing it!
On Mon, Oct 26, 2015 at 05:37 thefourtheye notifications@github.com wrote:

Closing this PR, as the patch landed in upstream. @mLuby
https://github.com/mLuby Thanks for trying to fix this :-)

—
Reply to this email directly or view it on GitHub
#3522 (comment).

@targos targos mentioned this pull request Nov 3, 2015
Closed
@aliscco aliscco mentioned this pull request Feb 11, 2024
Open
@aliscco aliscco mentioned this pull request Mar 23, 2024
Open
@aliscco aliscco mentioned this pull request May 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mLuby @jbergstroem @mscdex @thefourtheye