lix-project/lix-install-action
1
0
Fork 0
Code Issues 11 Pull requests Actions Packages Projects Releases Wiki Activity

Rebase on top of detsys-ts for abstracting over install.determinate.systems #74

Merged
grahamc merged 13 commits from detsys-ts into main 2024-04-11 15:58:56 +00:00
Conversation 0 Commits 13 Files changed 19 +96717 -17097
grahamc commented 2024-04-11 01:02:47 +00:00 (Migrated from github.com)
Copy link
Description
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)
##### Description <!--- Please include a short description of what your PR does and / or the motivation behind it ---> ##### 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)
lucperkins (Migrated from github.com) approved these changes 2024-04-11 13:46:08 +00:00
grahamc (Migrated from github.com) reviewed 2024-04-11 14:01:20 +00:00
package.json
grahamc (Migrated from github.com) commented 2024-04-11 14:01:15 +00:00
Copy link
    "detsys-ts": "github:DeterminateSystems/detsys-ts",
```suggestion "detsys-ts": "github:DeterminateSystems/detsys-ts", ```
colemickens (Migrated from github.com) reviewed 2024-04-11 14:12:22 +00:00
src/main.ts
@ -1089,28 +1021,20 @@ function action_input_bool(name: string): boolean {
return actions_core.getBooleanInput(name);
colemickens (Migrated from github.com) commented 2024-04-11 14:12:22 +00:00
Copy link

It looks like this is "finally"-ish enough to fire in case anything during install crashes, but maybe even just move it down to 1049, in an explicit finally?

It *looks* like this is "finally"-ish enough to fire in case anything during install crashes, but maybe even just move it down to 1049, in an explicit `finally`?
colemickens (Migrated from github.com) reviewed 2024-04-11 14:15:32 +00:00
src/main.ts
colemickens (Migrated from github.com) commented 2024-04-11 14:15:32 +00:00
Copy link

seems like getUniqueId already appears a randomUUID...

seems like getUniqueId already appears a randomUUID...
colemickens (Migrated from github.com) reviewed 2024-04-11 14:21:48 +00:00
.github/workflows/ci.yml
@ -17,2 +17,4 @@
- run: nix develop --command pnpm install
- run: nix develop --command pnpm run all
- run: git status --porcelain=v1
- run: test $(git status --porcelain=v1 2>/dev/null | wc -l) -eq 0
colemickens (Migrated from github.com) commented 2024-04-11 14:21:48 +00:00
Copy link

not sure if this applies to pnpm, but do you want/need some equivalent of --no-save? Seems like maybe that was just to avoid dirtying the tree mid-run?

not sure if this applies to pnpm, but do you want/need some equivalent of `--no-save`? Seems like maybe that was just to avoid dirtying the tree mid-run?
colemickens (Migrated from github.com) reviewed 2024-04-11 14:23:00 +00:00
action.yml
colemickens (Migrated from github.com) commented 2024-04-11 14:23:00 +00:00
Copy link

I realize these are renames of existing functionality, but it seems like source-url more or less can encompass all of these usages - preusmably you can "just" use a URL that points to the branch/pr/rev/tag?

I realize these are renames of existing functionality, but it seems like `source-url` more or less can encompass all of these usages - preusmably you can "just" use a URL that points to the branch/pr/rev/tag?
colemickens (Migrated from github.com) reviewed 2024-04-11 14:31:33 +00:00
package.json
@ -12,3 +13,4 @@
"all": "pnpm run format && pnpm run lint && pnpm run build && pnpm run package"
},
"repository": {
"type": "git",
colemickens (Migrated from github.com) commented 2024-04-11 14:31:33 +00:00
Copy link

run prettier too? or does lint check prettier? might be nice either way, if we check prettier on CI, to know locally first (not that I forget and have to push prettier fixup commits...)

run prettier too? or does lint check prettier? might be nice either way, if we check prettier on CI, to know locally first (not that I forget and have to push prettier fixup commits...)
colemickens (Migrated from github.com) approved these changes 2024-04-11 14:31:39 +00:00
grahamc (Migrated from github.com) reviewed 2024-04-11 15:45:31 +00:00
src/main.ts
@ -1089,28 +1021,20 @@ function action_input_bool(name: string): boolean {
return actions_core.getBooleanInput(name);
grahamc (Migrated from github.com) commented 2024-04-11 15:45:31 +00:00
Copy link

I've cleaned this up a lot, by delegating the actual execution of this code as callbacks that the IdsToolbox calls.

I've cleaned this up a lot, by delegating the actual execution of this code as callbacks that the IdsToolbox calls.
grahamc (Migrated from github.com) reviewed 2024-04-11 15:45:47 +00:00
.github/workflows/ci.yml
@ -17,2 +17,4 @@
- run: nix develop --command pnpm install
- run: nix develop --command pnpm run all
- run: git status --porcelain=v1
- run: test $(git status --porcelain=v1 2>/dev/null | wc -l) -eq 0
grahamc (Migrated from github.com) commented 2024-04-11 15:45:47 +00:00
Copy link

We're doing this deliberately actually, to make sure it is clean and not an outdated lockfile.

We're doing this deliberately actually, to make sure it is clean and not an outdated lockfile.
grahamc (Migrated from github.com) reviewed 2024-04-11 15:45:59 +00:00
package.json
@ -12,3 +13,4 @@
"all": "pnpm run format && pnpm run lint && pnpm run build && pnpm run package"
},
"repository": {
"type": "git",
grahamc (Migrated from github.com) commented 2024-04-11 15:45:59 +00:00
Copy link

format does that :)

`format` does that :)
grahamc (Migrated from github.com) reviewed 2024-04-11 15:52:38 +00:00
action.yml
grahamc (Migrated from github.com) commented 2024-04-11 15:52:38 +00:00
Copy link

We could, but I've seen time and time again folks (internal & external) mess up the structure, so I think having these sorts of helpers makes it a nicer experience for testing PRs and whatnot.

We could, but I've seen time and time again folks (internal & external) mess up the structure, so I think having these sorts of helpers makes it a nicer experience for testing PRs and whatnot.
Sign in to join this conversation.
Reviewers
No reviewers
lucperkins
colemickens
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix-install-action#74
No description provided.
Delete branch "detsys-ts"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?