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

chore: minor tuning of GHA build #3617

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ssbarnea
Copy link
Collaborator

@ssbarnea ssbarnea commented Oct 1, 2024

Description

This should address error seen on https://github.com/streetsidesoftware/cspell-dicts/actions/runs/11125090808/job/30912037317?pr=3446#step:3:67 which should have not happen, especially as the package.json and its lock were not touched by that change.

References

  • Any source references.

Checklist

  • By submitting this pull-request, you agree to follow our Code of Conduct
  • Verify that the title starts with the correct prefix:
    • fix: - for minor changes like adding words or fixing spelling issues.
    • feat: - for a significant change like adding a whole new set of words to a dictionary.
    • feat!: - for breaking changes, like file format or licensing changes.
    • chore: - for changes that do not impact the content of dictionaries.

@ssbarnea ssbarnea marked this pull request as ready for review October 1, 2024 12:44
Copy link
Collaborator

@Jason3S Jason3S left a comment

Choose a reason for hiding this comment

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

Thank you for the heads up on the broken build.

.tool-versions Outdated Show resolved Hide resolved
.github/actions/setup/action.yaml Outdated Show resolved Hide resolved
dictionaries/docker/source/Dockerfile Outdated Show resolved Hide resolved
@ssbarnea ssbarnea marked this pull request as draft October 2, 2024 10:45
@ssbarnea ssbarnea changed the title chore: Fix GHA pipelines by using a nodejs 22 chore: minor tuning of GHA build Oct 2, 2024
@ssbarnea
Copy link
Collaborator Author

ssbarnea commented Oct 2, 2024

I made it draft as I will use to modify the actions to make use of the file.

One thing that I observed is that pnpm has the bad habit of updating the lockfile by default on install, that being different than npm and yarn, which need an extra argument to refresh the lock. pnpm goes the other way around and requires the switch to make it use the file.

Based on version of node being used (18 or 20), it seems that it updates two types files, very unlikely to really break the code but enough to cause churn.

I guess that we need to be sure that the lock file is produced with minimal supported version of node.

@Jason3S
Copy link
Collaborator

Jason3S commented Oct 5, 2024

I made it draft as I will use to modify the actions to make use of the file.

One thing that I observed is that pnpm has the bad habit of updating the lockfile by default on install, that being different than npm and yarn, which need an extra argument to refresh the lock. pnpm goes the other way around and requires the switch to make it use the file.

Based on version of node being used (18 or 20), it seems that it updates two types files, very unlikely to really break the code but enough to cause churn.

I guess that we need to be sure that the lock file is produced with minimal supported version of node.

There is a workflow (update-dependencies) that will fix the pnpm-lock.yaml file. There is a bit of churn, but it settles on the workflow version.

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.

2 participants