test: fixup testing for specific N-API version #840
Conversation
Fixup testing for a specific N-API version and document.
|
@KevinEady, @legendecas I don't think the linting is working correctly. It's reporting problems in files I've not changed. |
|
@mhdawson the check is comparing the local master with the current PR branch. So on the CI, it will compare with the latest master. We have to update the PR branch with the latest master branch to make sure the diff do not include changes landed on master after the fork of the development branch. |
|
@legendecas I'm not following what we need to fix this? |
|
＠mhdawson Sorry but maybe I'm misunderstanding the problem. I didn't find the CI is failing. Can you help me explain what failure you're referring to and how to reproduce? |
|
@legendecas it was reporting failures when I ran locally and I had to add the comment to ingore the pre-commit checks. |
|
@mhdawson ok. The linter compares the current working branch with the local master branch. If the local master branch is not up to date or contains the latest commits that not linted, which commits the working branch doesn't include, then the diff will include those unrelated commits and the linter will complain about them. Keep local branches up to date will mediate the problem but since the checks depend on those diffs, it is not always reliable locally. However, branches on the CI will always be updated. |
|
@legendecas that may cause people confusion/trouble since I'm not sure how many people keep their local master branch up to date. |
|
@mhdawson FWIW, the script allows you to specify a process env |
|
The local master branch might not always tracking nodejs/node-addon-api but rather someones-fork/node-addon-/api so I didn't comparing with local upstream/master or some-local-remote/master. Maybe we can print instructions when the check failed how to fix the local check to improve the experience. |
Fixup testing for a specific N-API version and document. Reviewed-By: NickNaso <nicoladelgobbo@gmail.com> PR-URL: nodejs/node-addon-api#840
Fixup testing for a specific N-API version and document. Reviewed-By: NickNaso <nicoladelgobbo@gmail.com> PR-URL: nodejs/node-addon-api#840
Fixup testing for a specific N-API version and document. Reviewed-By: NickNaso <nicoladelgobbo@gmail.com> PR-URL: nodejs/node-addon-api#840
Fixup testing for a specific N-API version and document. Reviewed-By: NickNaso <nicoladelgobbo@gmail.com> PR-URL: nodejs/node-addon-api#840
Fixup testing for a specific N-API version
and document.