Fix CI references to spaces in package names #95
Conversation
|
I was tempted to merge but @weefuzzy should review maybe to avoid more misunderstandings... |
|
No trouble, now we have the PR checker in place which might guard against some low hanging fruit. |
.github/workflows/nightly.yaml
Outdated
| @@ -3,6 +3,8 @@ name: Nightly Releases | |||
| on: | |||
| push: | |||
| branches: [ dev, ci/automatic-building ] | |||
| pull_request: | |||
There was a problem hiding this comment.
I'd be inclined to keep things simple and just address the spaces issue in this PR, so nightly builds carry on working as before. Whether we really want to do a CI build for every PR warrants a bit of thought I think.
There was a problem hiding this comment.
I initially had it removed because I was not convinced 100% that it was necessary. I think, given the future may have more testing involved that we could have tests run on a PR in a similar fashion, as well as a build to see if it will work properly. A question mark to be resolved I reckon but for now let's see if it is useful?
There was a problem hiding this comment.
If you like; I guess it seems profligate in terms of launching lots and lots of builds (I can obviously see the utility though). In any case, not in the same merge commit as the fix for this issue would be preferable from a history-hygiene POV, IMO, YMMV, etc.
There was a problem hiding this comment.
Yeah okay. I'll cherry wangle this mofo into another one.
|
Also, yikes, that's a lot of places that the action needs to know the name of the package folder. We should probably imagine a future improvement to make that discoverable (not that I imagine it changing henceforth, but...) |
It is a consequence of building in 3 separate runners and then packaging again downstream into one zip folder. If there were a programmatic way to get the install output location and return it as a stdout variable then I think we could store it in an environment variable for the runner making it a bit more flexible. I would flag that as a low priority issue though... |
No description provided.