Skip to content

Github package CI#512

Closed
Intubun wants to merge 2 commits into
RTimothyEdwards:masterfrom
Intubun:master
Closed

Github package CI#512
Intubun wants to merge 2 commits into
RTimothyEdwards:masterfrom
Intubun:master

Conversation

@Intubun
Copy link
Copy Markdown
Contributor

@Intubun Intubun commented May 12, 2026

Hey,
Ive seen that the CI some how didnt trigger the package publish. Ive added a trigger that will trigger both on changes in the VERSION file and as tag push. @RTimothyEdwards you should change the settings under Settings -> Actions -> General -> Workflow permissions and set it to "Read and write permissions" in order for the package publishing to work as intended

Copy link
Copy Markdown
Owner

@RTimothyEdwards RTimothyEdwards left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@RTimothyEdwards
Copy link
Copy Markdown
Owner

Pulled and merged into opencircuitdesign.com. I updated the version so that it will force a test of the workflow overnight.

@Intubun
Copy link
Copy Markdown
Contributor Author

Intubun commented May 12, 2026

Have you changed the settings in github?

@RTimothyEdwards
Copy link
Copy Markdown
Owner

The settings are already set to "read and write permissions". Could it be something else?

@Intubun
Copy link
Copy Markdown
Contributor Author

Intubun commented May 12, 2026

Great! It just didnt trigger last time. At least in my fork it was reproducable and I fixed it. I just asked to make sure it is set the right way

Intubun pushed a commit to Intubun/magic that referenced this pull request May 13, 2026
…ow. Updated

the version to force a test of the workflow.
@Intubun
Copy link
Copy Markdown
Contributor Author

Intubun commented May 13, 2026

Update: It did work: https://github.com/RTimothyEdwards/magic/pkgs/npm/magic-vlsi-wasm The package can now be pulled by npm with a .npmrc config file inside a project. Not yet published on npm, which will happen when the package can be considered for a broader audience. @dlmiles

@dlmiles
Copy link
Copy Markdown
Collaborator

dlmiles commented May 13, 2026

@RTimothyEdwards It produces https://github.com/RTimothyEdwards/magic/pkgs/npm/magic-vlsi-wasm which can be seen on the right hand side of the main GitHub page just below Releases.

Does the NPM need a valid GITHUB_TOKEN to access the package ? If so this should be documented in the page example, as it saves support questions over this matter.

I'm not sure on the semantic versioning using the git commit hash as the 4th part is ideal. It would have been better to keep that free (available to use later). What do other packages do in this area ? 1.2.3-20260101-0123cdef ? Allowing 1.2.3.4-20260101-0123cdef to exist i the future, if necessary.

@Intubun
Copy link
Copy Markdown
Contributor Author

Intubun commented May 13, 2026

the github token is just for beta testers, as of now. Mostly since github is not in the standard sources for npm. Therefore it has to be manually added. I should probably add a note in the npm/README.md for early adopters. But I will review the choice of the versioning in the coming days.

@dlmiles
Copy link
Copy Markdown
Collaborator

dlmiles commented May 14, 2026

The specific issue is the use of the git-hash as the leading characters of a part of the version, as the hash is never lexically ordered or sortable in the way needed by semver, this is the secondary point of leading it with a date (which is lexically ordered and sortable).

Some project might use 1.2.3-YYYMMDDgit0123cdef where the 'git' helps convey what the id after means. The important thing is this character is not a full-stop '.' which has meaning in semantic version.

The point of the date is to prefix the git hash with something sortable that makes the entire string sortable, so the date part needs to have strong associativity with the git hash, not string associativity with the version patch level (3rd part).

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