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

Deprecate stack-yaml input #50

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Deprecate stack-yaml input #50

merged 4 commits into from
Feb 23, 2024

Conversation

pbrisbin
Copy link
Member

@pbrisbin pbrisbin commented Feb 21, 2024

Set isolatedModules: true

1a48f85

See https://stackoverflow.com/a/60905543

This brought the test suite down from ~6s to ~1s on my machine.

Commit forgotten README update

add9aad

Remove stack-yaml input

7d8662b

Users can either set STACK_YAML or stack-arguments to add
--stack-yaml, which each come with their own trade-offs depending on
the nature of the Job in which this action is used. Either way, a
dedicated input is not necessary or useful.

Before this commit, users who set STACK_YAML and used
inputs.stack-yaml could get themselves in trouble if they weren't
careful.

I decided to keep the input for now, to avoid needing a major version
bump but:

  • Documentation says it's deprecated
  • It triggers a warning
  • It results in naively pushing the value into stack-arguments, which
    is what the behavior was before, but might misbehave if the user is
    also setting env.STACK_YAML or already has --stack-yaml in
    stack-arguments, which was also the behavior before, I guess

README.md Outdated Show resolved Hide resolved
src/stack-cli.ts Outdated Show resolved Hide resolved
See https://stackoverflow.com/a/60905543

This brought the test suite down from ~6s to ~1s on my machine.
Users can either set `STACK_YAML` or `stack-arguments` to add
`--stack-yaml`, which each come with their own trade-offs depending on
the nature of the Job in which this action is used. Either way, a
dedicated input is not necessary or useful.

Before this commit, users who set `STACK_YAML` _and_ used
`inputs.stack-yaml` could get themselves in trouble if they weren't
careful.

I decided to keep the input for now, to avoid needing a major version
bump but:

- Documentation says it's deprecated
- It triggers a warning
- It results in naively pushing the value into `stack-arguments`, which
  is what the behavior was before, but might misbehave if the user is
  also setting `env.STACK_YAML` or already has `--stack-yaml` in
  `stack-arguments`, which was also the behavior before, I guess
@pbrisbin pbrisbin changed the title pb/rm stack yaml 2 Deprecate stack-yaml input Feb 21, 2024
@pbrisbin pbrisbin requested a review from stackptr February 21, 2024 14:27
@pbrisbin pbrisbin marked this pull request as ready for review February 21, 2024 15:40
@pbrisbin pbrisbin merged commit 9d997b5 into main Feb 23, 2024
18 checks passed
@pbrisbin pbrisbin deleted the pb/rm-stack-yaml-2 branch February 23, 2024 14:05
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.

2 participants