Skip to content

Add new packs input to init action - #591

Merged
aeisenberg merged 3 commits into
mainfrom
aeisenberg/pack-in-inputs
Jun 25, 2021
Merged

Add new packs input to init action#591
aeisenberg merged 3 commits into
mainfrom
aeisenberg/pack-in-inputs

Conversation

@aeisenberg

@aeisenbergaeisenberg commented Jun 23, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

This input allows users to specify which packs to run. It works in
unison with the packs block of the config file and it is similar to
how queries works. They both use + in the same way.

Note that the #TODO in the pr check is still around, but the CLI
is available. I will remove the TODO in the next commit.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@aeisenberg
aeisenberg requested a review from a team as a code ownerJune 23, 2021 22:42
@aeisenberg
Add new packs input to init action
6e577cf
This input allows users to specify which packs to run. It works in
unison with the packs block of the config file and it is similar to
how `queries` works. They both use `+` in the same way.
Note that the `#TODO` in the pr check is still around, but the CLI
is available. I will remove the TODO in the next commit.
@aeisenberg
aeisenbergforce-pushed the aeisenberg/pack-in-inputs branch from 108ced1 to 6e577cfCompareJune 23, 2021 23:08
edoardopirovano
edoardopirovano reviewed Jun 24, 2021
View reviewed changes

@edoardopirovanoedoardopirovano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! This mostly looks good, a few comments inline.

Comment thread.github/workflows/pr-checks.yml
Comment thread.github/workflows/pr-checks.yml
Comment thread.github/workflows/pr-checks.yml
Comment threadinit/action.yml Outdated
Comment threadsrc/config-utils.test.ts
Comment threadsrc/config-utils.ts Outdated
Comment threadsrc/config-utils.ts Outdated
Comment threadsrc/config-utils.ts Outdated
if (packsInput.startsWith("+")) {
packsInput = packsInput.substring(1).trim();
if (!packsInput) {
throw new Error("Remove the '+' from the packs input.");

@edoardopirovanoedoardopirovanoJun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could be a little more informative, e.g. A + was used in the packs input to specify that you wished to add some packs to your CodeQL analysis. However, no packs were specified. Please either remove the + or specify some packs.

@edoardopirovanoedoardopirovanoJun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aeisenberg Think you missed this comment? Unless you disagree, in which case no need to address it 🙂

@aeisenbergaeisenbergJun 24, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Right. Missed this. Thanks for pointing it out.

Comment threadsrc/runner.ts Outdated
Comment thread.github/workflows/pr-checks.yml
edoardopirovano
edoardopirovano approved these changes Jun 24, 2021
View reviewed changes
Comment thread.github/workflows/pr-checks.yml Outdated
- uses: ./../action/init
with:
config-file: ".github/codeql/codeql-config-packaging3.yml"
packs: dsp-testing/codeql-pack1@0.0.4

@edoardopirovanoedoardopirovanoJun 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you need a + here? Otherwise this LGTM now.

@aeisenbergaeisenbergJun 24, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Doh!

@aeisenberg
aeisenbergforce-pushed the aeisenberg/pack-in-inputs branch 2 times, most recently from 257ff39 to 76e2e21CompareJune 25, 2021 16:36
@aeisenberg

aeisenberg commented Jun 25, 2021

Copy link
Copy Markdown
ContributorAuthor

Build is failing because the version of the cli it is grabbing from the toolcache is not yet new enough to support all of the packaging commands.

@aeisenberg
Add extra integration test for packaging
4087f37
Also, update the options and inputs documentation.
@aeisenberg
aeisenbergforce-pushed the aeisenberg/pack-in-inputs branch from 76e2e21 to 4087f37CompareJune 25, 2021 17:08
@aeisenberg
aeisenberg merged commit 7726ece into mainJun 25, 2021
@aeisenberg
aeisenberg deleted the aeisenberg/pack-in-inputs branch June 25, 2021 17:52
@github-actionsgithub-actionsBot mentioned this pull request Jun 28, 2021
Merged
5 tasks
@alisccoaliscco mentioned this pull request Apr 27, 2023
Open
@CharliesAngel1CharliesAngel1 mentioned this pull request Nov 15, 2023
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@edoardopirovanoedoardopirovanoedoardopirovano approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aeisenberg@edoardopirovano