Skip to content
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.

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

chore(workflow): add release and test workflows #187

Closed
wants to merge 13 commits into from

Conversation

JamesSingleton
Copy link
Contributor

@JamesSingleton JamesSingleton commented Dec 10, 2020

In order to get this to pass, I needed to change the implies to demandOption

.implies({
  'parrot-middleware': 'modules',
  'dev-endpoints': 'modules',
})

to

demandOption: !require('yargs').argv.modules && require('yargs').argv.moduleMapUrl,

Basically what the implies does, is tell yargs that if parrot-middleware or dev-endpoints is provided, modules also needs to be provided. So changing it to demand the option when that was the case fixes the snapshot issue that GitHub Actions is having. For some reason GitHub actions is running yargs@14 at most.

@JamesSingleton JamesSingleton requested review from a team as code owners December 10, 2020 22:57
],
Array [],
Array [
"Unknown arguments: not-a-valid-option, notAValidOption",
Copy link
Contributor

Choose a reason for hiding this comment

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

This snapshot has changed with a different value, I assume it is because the new rule fails fast for --parrot-middleware before yargs sees the invalid arguments 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

might become a DX issue if not all arguments are printed in the "help" when an invalid argument is added (typo for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@infoxicator yeah was trying to figure out a way to get the tests to pass but maintain the same functionality.

with:
node-version: ${{ matrix.node }}
- name: Install Dependencies
run: npm ci && npm ls yargs && npm list -g --depth 0
Copy link
Contributor

Choose a reason for hiding this comment

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

was this npm ls yargs && npm list -g --depth 0 needed for the yargs issue with GH actions? can you point me to the link where the issue with the yargs version is explained?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes I need to remove that 🤦🏼 yargs/yargs#1317

@JamesSingleton
Copy link
Contributor Author

Going to decline this PR for now and wait and see if GitHub fixes their actions

@nellyk nellyk deleted the chore/update-to-github-actions branch June 8, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants