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

src: wrap source before doing syntax check #3587

Merged
merged 1 commit into from
Oct 29, 2015

Conversation

evanlucas
Copy link
Contributor

This is to ensure that it is evaluated the same way it would be if it
were to be run by node or required.

Before, the following would pass if run by node, but fail if run via
the syntax check flag:

if (true) {
  return;
}

Now, this will pass the syntax check

Reported in #2411 (comment)

@bnoordhuis
Copy link
Member

LGTM if CI etc.

@evanlucas
Copy link
Contributor Author

@thefourtheye
Copy link
Contributor

LGTM

This is to ensure that it is evaluated the same way it would be if it
were to be run by node or required.

Before, the following would pass if run by node, but fail if run via
the syntax check flag:

    if (true) {
      return;
    }

Now, this will pass the syntax check

PR-URL: nodejs#3587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@evanlucas evanlucas closed this Oct 29, 2015
@evanlucas evanlucas deleted the fixcheck branch October 29, 2015 18:10
@evanlucas evanlucas merged commit 08166cb into nodejs:master Oct 29, 2015
@evanlucas
Copy link
Contributor Author

Thanks, landed in 08166cb

@evanlucas
Copy link
Contributor Author

Do we want to try to get this into v5.0.0 too?

evanlucas added a commit that referenced this pull request Oct 29, 2015
This is to ensure that it is evaluated the same way it would be if it
were to be run by node or required.

Before, the following would pass if run by node, but fail if run via
the syntax check flag:

    if (true) {
      return;
    }

Now, this will pass the syntax check

PR-URL: #3587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
evanlucas added a commit that referenced this pull request Oct 29, 2015
This is to ensure that it is evaluated the same way it would be if it
were to be run by node or required.

Before, the following would pass if run by node, but fail if run via
the syntax check flag:

    if (true) {
      return;
    }

Now, this will pass the syntax check

PR-URL: #3587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 29, 2015

Landed in v4.x-staging in 7883294

@rvagg
Copy link
Member

rvagg commented Oct 30, 2015

possible semver-minor?

@evanlucas
Copy link
Contributor Author

I would think it's a bug fix, but if you feel differently, whatever works for me

@jasnell
Copy link
Member

jasnell commented Oct 30, 2015

I'd say bug fix.
On Oct 29, 2015 7:04 PM, "Rod Vagg" notifications@github.com wrote:

possible semver-minor?


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

@thefourtheye
Copy link
Contributor

+1 for bug fix.

evanlucas added a commit that referenced this pull request Nov 7, 2015
This is to ensure that it is evaluated the same way it would be if it
were to be run by node or required.

Before, the following would pass if run by node, but fail if run via
the syntax check flag:

    if (true) {
      return;
    }

Now, this will pass the syntax check

PR-URL: #3587
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Nov 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants