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

build: don't run lint from test-ci #1965

Closed

Conversation

jbergstroem
Copy link
Member

Since we will run linting before compiling or testing there's no need to run it as part of the ci testing.

Note to person landing this: Don't merge until we have the linter up and running (#1955 and nodejs/build#109).

Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.
@brendanashworth brendanashworth added the build Issues and PRs related to build files or the CI. label Jun 13, 2015
@thefourtheye
Copy link
Contributor

I am not sure if this is a good idea. What if people execute only the relevant test cases instead of make test? I feel that CI should have all the checks.

@rvagg
Copy link
Member

rvagg commented Jun 13, 2015

@thefourtheye the context for this is that we're splitting off linting to a separate pre-test in CI which can be run and fail quickly before even compiling.

@thefourtheye
Copy link
Contributor

Oh. Cool then :-)

@cjihrig
Copy link
Contributor

cjihrig commented Jun 13, 2015

LGTM. Running the linter first/separately sounds like a great idea.

@evanlucas
Copy link
Contributor

LGTM

jbergstroem added a commit that referenced this pull request Jun 15, 2015
Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.

PR-URL: #1965
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Revewied-By: Evan Lucas <evanlucas@me.com>
@jbergstroem
Copy link
Member Author

Merged in 8d8a26e. Thanks for the review.

@rvagg rvagg mentioned this pull request Jun 16, 2015
orangemocha pushed a commit to orangemocha/node that referenced this pull request Jun 17, 2015
Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.

Cherry-picked from nodejs/node@8d8a26e
Original commit metadata follows:
  PR-URL: nodejs/node#1965
  Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  Revewied-By: Evan Lucas <evanlucas@me.com>
orangemocha pushed a commit to orangemocha/node that referenced this pull request Jun 29, 2015
Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.

Cherry-picked from nodejs/node@8d8a26e
Original commit metadata follows:
  PR-URL: nodejs/node#1965
  Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  Revewied-By: Evan Lucas <evanlucas@me.com>

Reviewed-By: Dummy Reviewer <dummy@reviewer.org>
PR-URL: #11
rvagg pushed a commit to rvagg/io.js that referenced this pull request Jul 3, 2015
Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.

PR-URL: nodejs#1965
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Revewied-By: Evan Lucas <evanlucas@me.com>
jbergstroem added a commit that referenced this pull request Jul 3, 2015
Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.

PR-URL: #1965
PORT-PR-URL: #2101
PORT-FROM: 8d8a26e
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Revewied-By: Evan Lucas <evanlucas@me.com>
orangemocha pushed a commit to orangemocha/node that referenced this pull request Jul 7, 2015
Since we will run linting before compiling or testing there's no
need to run it as part of the ci testing.

Cherry-picked from nodejs/node@8d8a26e
Original commit metadata follows:
  PR-URL: nodejs/node#1965
  Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  Revewied-By: Evan Lucas <evanlucas@me.com>

Reviewed-By: Dummy Reviewer <dummy@reviewer.org>
PR-URL: #11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants