chore(action): Rework action to be a composite action #243

andreiborza · 2025-02-05T09:47:01Z

Removed docker image and workflow around it
Externalizing @sentry/cli so it can be installed per-runner
Support versioning of actions publishing major, minor and patch tags allowing users to pin versions
Reduce action run time by 80% as a result

- Removed docker image and workflow around it - Externalizing `@sentry/cli` so it can be installed per-runner - Support versioning of actions publishing major, minor and patch tags allowing users to pin versions - Reduce action run time by 80% as a result Closes: #185, #193, #233

…itten on the CI by the install step

mydea · 2025-02-05T12:15:58Z

.github/workflows/create-release-tags.yml

@@ -0,0 +1,35 @@
+  # Releases always publish a `v[major].[minor].[patch]` tag
+  # This action creates/updates `v[major]` and `v[major].[minor]` tags
+  name: Create release tags


I think we need special handling for pre-releases here? this should not run for those I guess. Ideally we only run this if the version matches ^(\d+)\.(\d+)\.(\d+)$, I suppose?

The action does not trigger on pre-releases, I figured we don't need extra tags for those?

ah, if that is the case, then all good!

.github/workflows/test.yml

mydea · 2025-02-05T12:17:19Z

.github/workflows/test.yml

-    - uses: './.github/actions/use-local-dockerfile'
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0


is this necessary? do we need all commits here?

This is a recommendation we give when using the action, probably because of set-commits. I suppose for the tests we don't need to.

Co-authored-by: Francesco Gringl-Novy <[email protected]>

mydea · 2025-02-05T12:23:26Z

action.yml

+
+    - name: Install Sentry CLI v2
+      shell: bash
+      run: yarn add @sentry/cli@2


l: For a follow up, let's allow to make this configurable as an input (optionally) so you can pick a specific version here!

m: Also, how does that work with caching & new cli versions? We should ensure that new v2 versions are used. Probably, we should cache yarn including the used sentry-cli version here? 🤔 and use npm view @sentry/cli version to get the latest v2 version here, somehow...?

Finally, instead of yarn add let's use yarn add @sentry/cli --no-lockfile --dev to avoid actually mutating the lockfile here, we do not care about this I'd say? that should also silence the warning for double installing this...?

Right.. I removed the caching again because it doesn't make sense when installing the cli on the ci and we can't possibly commit the lockfile back.

Replaced yarn add with yarn add --dev --no-lockfile @sentry/cli@2

Lms24

Sounds good and love the reduction in runtime!

Had a couple of questions but I think this is a great improvement!

Let's also make sure we document the new release flow properly so that future contributors know what to do. (if this is already done, even better; just didn't see anything in this PR)

Lms24 · 2025-02-05T12:51:10Z

.github/workflows/create-release-tags.yml

+            MINOR_VERSION=$(echo '${{ github.event.release.tag_name }}' | cut -d. -f1-2)
+            git tag -f $MAJOR_VERSION
+            git tag -f $MINOR_VERSION
+            git push -f origin $MAJOR_VERSION $MINOR_VERSION


just a question: this already handles previously set and pushed tags when we associate them with new commits?

Yes, tags will be force updated and force pushed so e.g. the v1 tag will be updated to point to the tag from the craft release e.g. v1.5.4

Lms24 · 2025-02-05T13:05:10Z

action.yml

+
+    - name: Install Sentry CLI v2
+      shell: bash
+      run: yarn add @sentry/cli@2


m: Have you thought about pinning the CLI version? Can we read it from dev dependencies? In the bundler plugins we pin it. I think it would be good to associate a specific release of the action with a specific CLI version.

I debatted pinning the version but people rarely upgrade their github actions and I figured it would be good to at least receive patches for sentry-cli.

I changed this to ~2.4 and in a follow up we might want to have this configurable.

good point that this isn't necessarily the same situation. I'm a bit concerned that this then still doesn't give us consistent behaviour for one action version but we can still change this later on if we ever receive bug reports. No objections to going with ^2.4 or ~2.4 then

mydea

🚀

mydea · 2025-02-06T12:54:39Z

src/main.ts

+      const workingDirectory = options.getWorkingDirectory();
+      const currentWorkingDirectory = process.cwd();
+
+      if (workingDirectory !== null && workingDirectory.length > 0) {


l: Can this just be if (workingDirectory) ?

andreiborza force-pushed the ab/composite-action branch from 02af32a to aa35025 Compare February 5, 2025 09:48

andreiborza force-pushed the ab/composite-action branch from aa35025 to 13bddeb Compare February 5, 2025 09:50

andreiborza added 2 commits February 5, 2025 12:18

Fix path

5e197bd

Add @sentry/cli as dev dep so we can build locally but have it overwr…

c6d6b15

…itten on the CI by the install step

andreiborza requested review from mydea and Lms24 February 5, 2025 11:43

Improve changelog

d903ebf

mydea reviewed Feb 5, 2025

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

mydea reviewed Feb 5, 2025

View reviewed changes

Update .github/workflows/test.yml

92c305a

Co-authored-by: Francesco Gringl-Novy <[email protected]>

mydea reviewed Feb 5, 2025

View reviewed changes

Lms24 approved these changes Feb 5, 2025

View reviewed changes

andreiborza added 3 commits February 5, 2025 14:16

Remove caching of yarn cache

640dd4d

Remove volta in action itself

a0def43

Remove fetch depths for ci tests

9b2d6dc

mydea approved these changes Feb 5, 2025

View reviewed changes

andreiborza added 2 commits February 5, 2025 14:40

Pin @sentry/cli to ~2.4

9029bd2

Change to ^2.4

c3e1fdc

andreiborza force-pushed the ab/composite-action branch from c671e2c to c3e1fdc Compare February 5, 2025 15:22

Read out workspace path directly in action

ea32e3d

andreiborza force-pushed the ab/composite-action branch from fcaeb81 to ea32e3d Compare February 5, 2025 15:55

Add comment

d4aaaaa

andreiborza force-pushed the ab/composite-action branch from e7f8815 to d4aaaaa Compare February 5, 2025 16:01

andreiborza added 2 commits February 5, 2025 17:10

Run lint and format

9613ff9

Add lint step to test.yml

7f9cbfc

andreiborza force-pushed the ab/composite-action branch from e5f8a48 to 7f9cbfc Compare February 5, 2025 16:12

mydea reviewed Feb 6, 2025

View reviewed changes

Simplify workingDirectory checks

32140db

andreiborza force-pushed the ab/composite-action branch from 7a80e3e to 32140db Compare February 6, 2025 13:01

andreiborza merged commit 24fbab5 into master Feb 6, 2025
15 checks passed

andreiborza deleted the ab/composite-action branch February 6, 2025 13:08

kirbysayshi mentioned this pull request Feb 13, 2025

Execution time has 2x-ed since becoming a composite action #261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(action): Rework action to be a composite action #243

chore(action): Rework action to be a composite action #243

andreiborza commented Feb 5, 2025

mydea Feb 5, 2025

andreiborza Feb 5, 2025

mydea Feb 5, 2025

mydea Feb 5, 2025

andreiborza Feb 5, 2025

andreiborza Feb 5, 2025

mydea Feb 5, 2025 •

edited

Loading

andreiborza Feb 5, 2025

Lms24 left a comment

Lms24 Feb 5, 2025

andreiborza Feb 5, 2025

Lms24 Feb 5, 2025 •

edited

Loading

andreiborza Feb 5, 2025

Lms24 Feb 5, 2025

mydea left a comment

mydea Feb 6, 2025

andreiborza Feb 6, 2025

chore(action): Rework action to be a composite action #243

chore(action): Rework action to be a composite action #243

Conversation

andreiborza commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mydea Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lms24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lms24 Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mydea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mydea Feb 5, 2025 •

edited

Loading

Lms24 Feb 5, 2025 •

edited

Loading