Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

npm publishing and deploying to cjquines.com #204

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

pihart
Copy link
Collaborator

@pihart pihart commented Jun 20, 2021

Resolves #117
Resolves #200 (6603030)
Resolves #201 (04f6cb6)

The npm publish workflow requires the NPM_TOKEN secret to be set to the token received from npm. It will do the build, tests (if present), and publish to npm on the creation of a GitHub release.

The semver workflow listens to merged pull requests with the semver: ***** labels and executes the appropriate npm commands to increment the package version according to the semver change level. It will push this change as a commit. This also creates a git tag, so creating the GitHub release is as simple as (on GitHub web) clicking into the tag and clicking the appropriate button.

I've thought about this quite a bit and feel somewhat strongly that semvers should be based on module compatibility. That is, the level of changes in the public API of any file reflect the semver level. It is reasonable to add a few exceptions in specific cases: if we drastically change the UI even without breaking any APIs, or if we break a large number of JSON files in a meaningful way (usually this also affects the public APIs so it's not a special case), we'd want to create a major version just for developer convenience.

This does not preclude us from having a different semver pattern for the UI, localStorage format, or whatever else; in fact, we should still be doing those.

NOTE: You typically don't want to merge multiple pull requests with such a label immediately after each other. This is because this workflow tries to push a commit, so if workflows are running in parallel, only the first to complete will actually be allowed to push the commit (we don't ever force push). If updating the version for each pull request is important, just wait for the commit from the first PR to be pushed before merging the second PR. Alternatively, if it's only important that a certain version is given after all are merged (which seems to almost always be the case for this repo), just use the semver: no-semver label on all but the highest semver change; this prevents the semver workflow from running on them, so you can merge at whatever speed. (You should still have the semver: ****** labels for documentation.)

Especially once this gets added to the project, please don't push directly to the main branch; always working with PRs allows us to use CI, this semver, and of course reviews. Further, it helps with grouping changes together.

@pihart pihart added semver: minor SemVer represents module (i.e. npm published) compatibility semver: patch SemVer represents module (i.e. npm published) compatibility and removed semver: minor SemVer represents module (i.e. npm published) compatibility labels Jun 20, 2021
@pihart pihart requested a review from cjquines June 20, 2021 21:12
@pihart pihart changed the title npm publishing npm publishing and deploying to cjquines.com Jan 9, 2022
echo "${{ secrets.SSH_PRIVATE_KEY }}" > ~/.ssh/id_rsa
echo "${{ secrets.SSH_KNOWN_HOSTS }}" > ~/.ssh/known_hosts
- name: Publish
run: rsync -a dist/ ssh.phx.nearlyfreespeech.net:/home/public/qboard/
Copy link
Owner

Choose a reason for hiding this comment

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

i don't think this directly works—post-build, the hrefs in index.html are relative to /, which is incorrect, i think? we might need to add a homepage to package.json or do something else to get the right hrefs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The hrefs/srcs should(?) all be flat; see https://github.com/pihart/qboard/blob/gh-pages/index.html

Copy link
Owner

Choose a reason for hiding this comment

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

ok. i think the rsync might want a --delete. but i'll test it. also, i think this actually needs a username as part of the secrets xp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch SemVer represents module (i.e. npm published) compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React types should be a proper dependency Node types should not be used Publish as npm dependency
2 participants