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

test: linting for buffer-free-callback test by Trott · Pull Request #3230 · nodejs/node · GitHub

/ node Public

test: linting for buffer-free-callback test - #3230

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:fix-d1f24044
Closed

test: linting for buffer-free-callback test#3230
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:fix-d1f24044

Conversation

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member

Test added in d1f2404 does not pass linting rule added in 3de353b.
Fixed in this commit. common module required in all tests except
those that intentionally leak variables.

Fixes: #3229

Test added in d1f2404 does not pass linting rule added in 3de353b.
Fixed in this commit. `common` module required in all tests except
those that intentionally leak variables.

Fixes: nodejs#3229

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member Author

Trott added the test Issues and PRs related to the tests. label Oct 7, 2015

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member Author

/cc @indutny This minor change to your test look good to you?

Copy link
Copy Markdown
Contributor

LGTM

indutny commented Oct 7, 2015

Copy link
Copy Markdown
Member

@Trott I didn't see any errors when committing it. Are you sure that it is required for addons?

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member Author

@indutny Yes, I'm postive.

$ git checkout  d1f2404 
Note: checking out 'd1f2404'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b new_branch_name

HEAD is now at d1f2404... buffer: FreeCallback should be tied to ArrayBuffer
$ make jslint
./node tools/eslint/bin/eslint.js src lib test tools/eslint-rules \
        --rulesdir tools/eslint-rules --reset --quiet

test/addons/buffer-free-callback/test.js
  1:0  error  Mandatory module "common" must be loaded  required-modules

✖ 1 problem (1 error, 0 warnings)

make: *** [jslint] Error 1
$

indutny commented Oct 7, 2015

Copy link
Copy Markdown
Member

Ok, LGTM then.

indutny commented Oct 7, 2015

Copy link
Copy Markdown
Member

Shouldn't it go to other addons too?

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member Author

Yes, all the previously existing addon tests that didn't use common got it in c78091d which is the commit immediately prior to the linting rule.

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member Author

Because this change is so small and CI will fail until it lands, I'm going to go ahead and land it right away.

Trott added a commit that referenced this pull request Oct 7, 2015
Test added in d1f2404 does not pass linting rule added in 3de353b.
Fixed in this commit. `common` module required in all tests except
those that intentionally leak variables.

PR-URL: #3230
Fixes: #3229
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Trott commented Oct 7, 2015

Copy link
Copy Markdown
Member Author

Landed in 6f63a4a

Trott closed this Oct 7, 2015
jasnell mentioned this pull request Oct 8, 2015
29 tasks
Trott added a commit that referenced this pull request Oct 8, 2015
Test added in d1f2404 does not pass linting rule added in 3de353b.
Fixed in this commit. `common` module required in all tests except
those that intentionally leak variables.

PR-URL: #3230
Fixes: #3229
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Trott deleted the fix-d1f24044 branch January 13, 2022 22:28
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL