FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: wrap source before doing syntax check by evanlucas · Pull Request #3587 · nodejs/node · GitHub

/ node Public

src: wrap source before doing syntax check - #3587

Merged
evanlucas merged 1 commit into
nodejs:masterfrom
evanlucas:fixcheck
Oct 29, 2015
ConversationCommits (1)ChecksFiles changed
Merged

src: wrap source before doing syntax check#3587
evanlucas merged 1 commit into
nodejs:masterfrom
evanlucas:fixcheck

Conversation

evanlucas commented Oct 29, 2015

Copy link
Copy Markdown
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 commented Oct 29, 2015

Copy link
Copy Markdown
Member

LGTM if CI etc.

evanlucas commented Oct 29, 2015

Copy link
Copy Markdown
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/649/

thefourtheye commented Oct 29, 2015

Copy link
Copy Markdown
Contributor

LGTM

src: wrap source before doing syntax check
08166cb 
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 closed this Oct 29, 2015
evanlucas deleted the fixcheck branch October 29, 2015 18:10
evanlucas merged commit 08166cb into nodejs:master Oct 29, 2015

evanlucas commented Oct 29, 2015

Copy link
Copy Markdown
Contributor Author

Thanks, landed in 08166cb

evanlucas commented Oct 29, 2015

Copy link
Copy Markdown
Contributor Author

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

jasnell mentioned this pull request Oct 29, 2015
Closed
evanlucas added a commit that referenced this pull request Oct 29, 2015
src: wrap source before doing syntax check
7883294 
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
src: wrap source before doing syntax check
5d1f1c5 
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 commented Oct 29, 2015

Copy link
Copy Markdown
Member

Landed in v4.x-staging in 7883294

rvagg commented Oct 30, 2015

Copy link
Copy Markdown
Member

possible semver-minor?

evanlucas commented Oct 30, 2015

Copy link
Copy Markdown
Contributor Author

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

jasnell commented Oct 30, 2015

Copy link
Copy Markdown
Member

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 commented Oct 30, 2015

Copy link
Copy Markdown
Contributor

+1 for bug fix.

Fishrock123 mentioned this pull request Nov 6, 2015
Closed
evanlucas added a commit that referenced this pull request Nov 7, 2015
src: wrap source before doing syntax check
65cd03c 
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 mentioned this pull request Nov 11, 2015
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL