Skip to content

build: do not lint fixtures in make lint-md#22549

Closed
joyeecheung wants to merge 1 commit intonodejs:masterfrom
joyeecheung:lint-skip-fixtures
Closed

build: do not lint fixtures in make lint-md#22549
joyeecheung wants to merge 1 commit intonodejs:masterfrom
joyeecheung:lint-skip-fixtures

Conversation

@joyeecheung
Copy link
Copy Markdown
Member

@joyeecheung joyeecheung commented Aug 27, 2018

This is useful if we need to put markdown files in test/fixtures (I was working on pulling the WPT there)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 27, 2018

@joyeecheung build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/694/pipeline

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Aug 27, 2018
@Trott
Copy link
Copy Markdown
Member

Trott commented Aug 28, 2018

I would prefer no changes to line 1093 and adding -not -path 'test/fixtures/*' to line 1096.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 31, 2018
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 2, 2018
@addaleax
Copy link
Copy Markdown
Member

addaleax commented Sep 2, 2018

Removing author-ready because I kinda think addressing @Trott’s comment is worth it and this isn’t really all that urgent (I think)

@BridgeAR
Copy link
Copy Markdown
Member

BridgeAR commented Sep 5, 2018

Ping @joyeecheung

@joyeecheung
Copy link
Copy Markdown
Member Author

joyeecheung commented Sep 15, 2018

Updated to use the -not option instead.

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

@Trott
Copy link
Copy Markdown
Member

Trott commented Sep 15, 2018 •
edited by refack
Loading

Everything is good on CI except a known Windows flake. Windows re-run: https://ci.nodejs.org/job/node-test-commit-windows-fanned/20733/ (Update: ✔️)

@refack refack added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 15, 2018
@refack
Copy link
Copy Markdown
Contributor

refack commented Sep 15, 2018

Resume CI (with a special Windows trick): https://ci.nodejs.org/job/node-test-commit/21481/

@joyeecheung
Copy link
Copy Markdown
Member Author

joyeecheung commented Sep 16, 2018

Windows is still pretty red..

@refack
Copy link
Copy Markdown
Contributor

refack commented Sep 16, 2018

So again: https://ci.nodejs.org/job/node-test-pull-request/17219/

@Trott
Copy link
Copy Markdown
Member

Trott commented Sep 17, 2018

Only red was Windows fanned. Known flaky. Here's a re-run: https://ci.nodejs.org/job/node-test-commit-windows-fanned/20753/

@addaleax
Copy link
Copy Markdown
Member

addaleax commented Sep 17, 2018

… and again: https://ci.nodejs.org/job/node-test-commit-windows-fanned/20782/

@joyeecheung
Copy link
Copy Markdown
Member Author

joyeecheung commented Sep 17, 2018

Windows build is (finally) green

@joyeecheung
Copy link
Copy Markdown
Member Author

joyeecheung commented Sep 17, 2018 •
edited
Loading

Landed in bb9d788, thanks!

joyeecheung added a commit that referenced this pull request Sep 17, 2018
@joyeecheung
build: do not lint fixtures in make lint-md
bb9d788 
PR-URL: #22549
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
targos pushed a commit that referenced this pull request Sep 18, 2018
@joyeecheung @targos
build: do not lint fixtures in make lint-md
6f99252 
PR-URL: #22549
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@targos targos mentioned this pull request Sep 18, 2018
Merged
targos pushed a commit that referenced this pull request Sep 19, 2018
@joyeecheung @targos
build: do not lint fixtures in make lint-md
e07e573 
PR-URL: #22549
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
targos pushed a commit that referenced this pull request Sep 20, 2018
@joyeecheung @targos
build: do not lint fixtures in make lint-md
3fcd54f 
PR-URL: #22549
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

+2 more reviewers

@refack refack refack approved these changes

@danbev danbev danbev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@joyeecheung @nodejs-github-bot @Trott @addaleax @BridgeAR @refack @danbev @jasnell