Skip to content

Update contributing guide #492

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.

Sign up for GitHub

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

Merged
aeisenberg merged 1 commit into from
May 10, 2021
Merged

Update contributing guide #492

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ Before you start, ensure that you have a recent version of node installed. You c

This project also includes configuration to run tests from VSCode (with support for breakpoints) - open the test file you wish to run and choose "Debug AVA test file" from the Run menu in the Run panel.

You may want to run `tsc --watch` from the command line or inside of vscode in order to ensure build artifacts are up to date as you are working.

### Checking in compiled artifacts and `node_modules`

Because CodeQL Action users consume the code directly from this repository, and there can be no build step during an GitHub Actions run, this repository contains all compiled artifacts and node modules. There is a PR check that will fail if any of the compiled artifacts are not up to date. Compiled artifacts are stored in the `lib/` folder. For all day-to-day development purposes, this folder can be ignored.

Avoid running `npm install` (the node moddules should be up to date when you check out anyway) and instead use `npm ci` when you want to update dependencies `package.json`. If you make any changes to the `node_modules` folder , you must run `npm run removeNPMAbsolutePaths` to clean the folder and remove any locally identifying data. There is a PR check to ensure that this command has been run.
Copy link
Contributor

@robertbrignull robertbrignull May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but I'm not sure this information is correct. Please do correct me if I'm talking rubbish here.

If you want to add or upgrade a dependency then you do need to run npm install. If you use npm ci here it'll complain that the package.json and package-lock.json files don't match. I believe the true use of npm ci is to efficiently populate the node_modules directory from a package-lock.json file, but we're using it instead just to normalise things to a known state.

Copy link
Contributor Author

@aeisenberg aeisenberg May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Darn...I think you're right. Apologies for over-eager merging. I'll create a new PR to clarify.


### Running the action

To see the effect of your changes and to test them, push your changes in a branch and then look at the [Actions output](https://github.com/github/codeql-action/actions) for that branch. You can also exercise the code locally by running the automated tests.
Expand Down