Skip to content

Create check-dist.yml - #604

Merged
brcrista merged 3 commits into
mainfrom
brcrista/check-dist
Aug 5, 2021
Merged

brcrista merged 3 commits into
mainfrom
brcrista/check-dist

Conversation

@brcrista

Copy link
Copy Markdown
Contributor

This adds a workflow to check the contents of the checked-in dist/index.js against the expected version.

See actions/upload-artifact#227 for background and rationale.

@brcrista
brcrista requested a review from a team as a code owner June 25, 2021 15:35
@konradpabjan

konradpabjan commented Jun 25, 2021 •

Copy link
Copy Markdown
Contributor

Forgot, but certain first party actions have pre and post steps so there are 2 index.js files

This will need some tweaks to take into account dist/restore/index.js and dist/save/index.js

@brcrista

Copy link
Copy Markdown
Contributor Author

The run failed, dist/ has a different format than I expected 😢 https://github.com/actions/cache/tree/main/dist

@shrink shrink mentioned this pull request Jun 25, 2021
3 tasks done
@dhadka

dhadka commented Aug 5, 2021

Copy link
Copy Markdown
Contributor

Just FYI, we already do something like this -

- name: Ensure dist/ folder is up-to-date
, though I'm fine switching over so we have a consistent check for both actions.

@brcrista
brcrista merged commit c9db520 into main Aug 5, 2021
@brcrista
brcrista deleted the brcrista/check-dist branch August 5, 2021 18:39
@brcrista

brcrista commented Aug 5, 2021

Copy link
Copy Markdown
Contributor Author

Whoops, sorry @dhadka I didn't see your comment before I merged. I'm adding the same(-ish) workflow for all actions (see https://github.com/github/c2c-actions-service/issues/2382).

I actually think the existing implementation is a bit cleaner -- didn't occur to me that you don't have to mv the file before calling git diff. I think it makes sense as its own workflow, though. I'll make a follow-up PR.

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.

3 participants