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

fix: fix set-output #249

Merged
merged 11 commits into from
Feb 10, 2023
Merged

fix: fix set-output #249

merged 11 commits into from
Feb 10, 2023

Conversation

CamiloGarciaLaRotta
Copy link
Owner

@CamiloGarciaLaRotta CamiloGarciaLaRotta commented Feb 8, 2023

What

Attempt to fix #247

If you look at the Checks tab, we should see no set-output deprecation warning.

How

Quite a bunch of changes:

  • realized the npm run all script which generated a new dist/index.js (file used by GH Actions itself) was not working. The fix was a mix of:
    • moving to yarn (fixing some cyclical dependencies)
    • fixing tests/linters as all dependency versions were bumped by quite a bit
  • fix integrations tests
  • skip linting warning for the time being

cc @rvanderlinden for visibility

@CamiloGarciaLaRotta CamiloGarciaLaRotta force-pushed the fix/set-output branch 2 times, most recently from 2f7d342 to ed717ba Compare February 8, 2023 17:38
@CamiloGarciaLaRotta CamiloGarciaLaRotta force-pushed the fix/set-output branch 7 times, most recently from 08c2caa to 8d4a34b Compare February 8, 2023 21:13
process.env['INPUT_URL'] = 'ftp://>invalid|url<'
process.env['INPUT_URL'] = 'abc'
Copy link
Owner Author

Choose a reason for hiding this comment

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

unsure why, but tests with an ftp url just hanged indefinetively....

package.json Outdated
"pack": "ncc build",
"single-file": "ncc build",
Copy link
Owner Author

Choose a reason for hiding this comment

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

yarn pack is a built in command, so I renamed it to avoid conflicts

@@ -22,11 +22,14 @@ export function graphqlPayloadFor(
): string {
const query = minify(rawQuery)
const minifiedVariables = minify(rawVariables)
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
Copy link
Owner Author

@CamiloGarciaLaRotta CamiloGarciaLaRotta Feb 9, 2023

Choose a reason for hiding this comment

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

I'll open follow up issues for all of these manual overrides. At the moment I just want to make sure the release pipeline works as expected

#252

Comment on lines -187 to +188
it('should handle missing input gracefully', async () => {
it.skipWindows('should handle missing input gracefully', async () => {
Copy link
Owner Author

Choose a reason for hiding this comment

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

see #251 as to why we had to skip on Windows

@CamiloGarciaLaRotta CamiloGarciaLaRotta force-pushed the fix/set-output branch 3 times, most recently from 5e49c0d to bfc88eb Compare February 10, 2023 21:58
@CamiloGarciaLaRotta CamiloGarciaLaRotta merged commit 55bba1b into main Feb 10, 2023
@CamiloGarciaLaRotta CamiloGarciaLaRotta deleted the fix/set-output branch February 10, 2023 22:30
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.

Update the set-output command
1 participant