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

ci(build): upload compiled artifact & display job summary #127

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

uncenter
Copy link
Member

@uncenter uncenter commented Sep 9, 2024

I think this means we can view the sizes of the artifacts to make sure nothing like https://togithub.com/wxt-dev/wxt/issues/738 happens again. Also just means you can verify/test the files in general.

@uncenter uncenter requested a review from sgoudham September 9, 2024 00:42
@uncenter
Copy link
Member Author

uncenter commented Sep 9, 2024

There are three zips that I've globbed for (firefox, chrome, and "sources" - the repo contents basically - would be built by the previous steps), I assume they will each be uploaded as a separate artifact? Or are they combined into one zip artifact?

@sgoudham
Copy link
Contributor

sgoudham commented Sep 9, 2024

I assume they will each be uploaded as a separate artifact? Or are they combined into one zip artifact?

Unfortunately, you always get a wrapper zip with the artifact upload.
ref: actions/upload-artifact#39

@uncenter
Copy link
Member Author

uncenter commented Sep 9, 2024

I assume they will each be uploaded as a separate artifact? Or are they combined into one zip artifact?

Unfortunately, you always get a wrapper zip with the artifact upload. ref: actions/upload-artifact#39

Argh. Should I upload the directories of the zips isntead of the zip themselves, and individually?

@uncenter
Copy link
Member Author

uncenter commented Sep 9, 2024

Ex:

      - name: Upload compiled Chrome extension
        uses: actions/upload-artifact@v4
        with:
          path: dist/chrome-mv3

      - name: Upload compiled Firefox extension
        uses: actions/upload-artifact@v4
        with:
          path: dist/firefox-mv2

Seems cumbersome :/

@sgoudham
Copy link
Contributor

sgoudham commented Sep 9, 2024

I don't mind the wrapper zip these days and just accept the harsh reality that we live in lol.

You can separate it out into multiple steps if you want, I'd say it's fine in this case cause it's 3 known zips that you're building. It's a lot harder to justify something like all the accents of a Catppuccin flavour.

@uncenter
Copy link
Member Author

uncenter commented Sep 9, 2024

Do you know what the assets look like in the UI if you don't provide a name? Does it just use the filename? https://github.com/actions/upload-artifact#inputs says name defaults to artifact so does it just say "artifact"? The latter seems like the worst option so I hope not!

@sgoudham
Copy link
Contributor

sgoudham commented Sep 9, 2024

I think it does default to artifact, the examples across the organisation all try to provide a name of some sort: https://github.com/search?q=org%3Acatppuccin%20actions%2Fupload-artifact&type=code

@uncenter
Copy link
Member Author

uncenter commented Sep 9, 2024

CleanShot 2024-09-08 at 21 22 59

Alright I think this does what I want! Turns out you can add a job summary, so I used that to list the sizes of the output directories (helpful for the sources zip mostly, which I chose not to zip since it is kinda large).

@uncenter uncenter changed the title ci(build): upload compiled artifact ci(build): upload compiled artifact & display job summary Sep 9, 2024
@uncenter uncenter merged commit 9d4828d into main Sep 9, 2024
2 checks passed
@uncenter uncenter deleted the ci/artifact branch September 9, 2024 01:26
Copy link
Contributor

@sgoudham sgoudham left a comment

Choose a reason for hiding this comment

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

👍

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