Skip to content

Skip complicated rewrites, record groups.#104

Merged
grahamc merged 14 commits intomainfrom
grahamc/skip-complicated-rewrite
Jul 3, 2025
Merged

Skip complicated rewrites, record groups.#104
grahamc merged 14 commits intomainfrom
grahamc/skip-complicated-rewrite

Conversation

@grahamc
Copy link
Copy Markdown
Member

@grahamc grahamc commented Jul 2, 2025

Description

The previous version of this code would do fairly complicated parsing and restructuring server-side. That has been a big mistake. This change makes the manipulation happen client-side and send essentially the direct events we need to send.

One problem is the old method used the repository as the "distinct id" which is not right, and causes confusion between github repos and flakehub users. Now we're using anon distinct IDs which are random, and storing the repository as a group instead.

This also starts tracking the github action name as $app_name like we should have been all along.

Checklist
  • Tested changes against a test repository
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • (If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)

@netlify
Copy link
Copy Markdown

netlify bot commented Jul 2, 2025

Deploy Preview for detsys-ts-docs ready!

Name Link
🔨 Latest commit b4aa0bf
🔍 Latest deploy log https://app.netlify.com/projects/detsys-ts-docs/deploys/68668260dcddbf0008ba7acf
😎 Deploy Preview https://deploy-preview-104--detsys-ts-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

grahamc added 2 commits July 3, 2025 08:21
It used to expect server-side munging, but that caused a ton of problems. This unifies that behavior.
@grahamc grahamc force-pushed the grahamc/skip-complicated-rewrite branch from f011601 to b9bf2cb Compare July 3, 2025 12:27
src/index.ts Outdated
this.archOs = platform.getArchOs();
this.nixSystem = platform.getNixPlatform(this.archOs);

this.facts["$app_name"] = `${this.actionOptions.name}/action`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

JS lets us use $ in identifiers, so the quoting isn't needed here.

Suggested change
this.facts["$app_name"] = `${this.actionOptions.name}/action`;
this.facts.$app_name = `${this.actionOptions.name}/action`;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

some primal part of my PHP-addled brain HATES this.

@grahamc grahamc enabled auto-merge (squash) July 3, 2025 13:16
@grahamc grahamc merged commit 0095c47 into main Jul 3, 2025
5 checks passed
@grahamc grahamc deleted the grahamc/skip-complicated-rewrite branch July 3, 2025 13:17
@grahamc grahamc temporarily deployed to detsys-pr-bot July 3, 2025 13:17 — with GitHub Actions Inactive
@grahamc grahamc temporarily deployed to detsys-pr-bot July 3, 2025 13:17 — with GitHub Actions Inactive
@grahamc grahamc temporarily deployed to detsys-pr-bot July 3, 2025 13:17 — with GitHub Actions Inactive
@grahamc grahamc temporarily deployed to detsys-pr-bot July 3, 2025 13:17 — with GitHub Actions Inactive
@grahamc grahamc temporarily deployed to detsys-pr-bot July 3, 2025 13:17 — with GitHub Actions Inactive
@grahamc grahamc temporarily deployed to detsys-pr-bot July 3, 2025 13:17 — with GitHub Actions Inactive
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