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: get input from @actions/core instead of env #11

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

tony19
Copy link
Contributor

@tony19 tony19 commented Jul 22, 2022

Changes

  • Read inputs with @actions/core package instead of from env vars. This reduces config duplication in yaml and avoids manual serialization of inputs.
  • Assert that src/index.ts is running in GitHub action

@tony19 tony19 marked this pull request as ready for review July 22, 2022 05:07
@tony19 tony19 force-pushed the feat/inputs-from-actions-core branch from 6ef460b to 8eef1a5 Compare July 22, 2022 05:32
@nozomuikuta
Copy link
Contributor

@tony19

#10 has been merged, so would you update this PR as well by resolving the conflicts?

@tony19

This comment was marked as resolved.

package.json Show resolved Hide resolved
assert(!!process.env.UPSTREAM_REPO, '`upstreamRepo` is required.')
assert(!!process.env.HEAD_REPO, '`headRepo` is required.')
assert(!!process.env.TRACK_FROM, '`trackFrom` is required.')
assert(typeof core !== 'undefined', `core is undefined, which probably means you're not running in a GitHub Action`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this assertion mean that we can no longer test the whole logic in our local environments?
If so, how we can test its behavior before release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I don't recall. This PR is so old, I've completely forgotten the context of this code, but I think this came from example code somewhere. I'll have to dig to find out the reason for it.

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