Make build and testing more predictable

[ssbarnea]

• Added missing files to .gitignore and added preventive measure to GHA

• Added missing dev dependency pnpm

• Prevent accidental use of npm install or incorrect versions of yarn

• This should make easier to contribute to the project

• No need to update changelog as there are no changes to the code being shipped

• Run tests

• Update the CHANGELOG.md with a summary of your changes

[ssbarnea]

@ntotten Any chance you can take a look at this one?

[github-actions]

This pull request has been labeled as stale due to inactivity. Reply to keep this pull request open.

[Copilot]

Pull request overview

This PR aims to improve build and testing predictability by enforcing Yarn usage, adding package manager dependencies, updating .gitignore, and adding CI checks for repository cleanliness. However, the implementation has several critical issues that actually undermine the goal of predictability, particularly the decision to ignore yarn.lock and .yarnrc.yml files.

Key issues identified:

• Critical: Adding yarn.lock to .gitignore contradicts the goal of predictable builds
• Critical: Cross-platform compatibility issues with bash-only scripts
• Multiple workflow commands that won't fail builds when they should

Reviewed changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated 5 comments.

File	Description
.gitignore	Adds entries for yarn configuration files and install state, but problematically ignores yarn.lock and .yarnrc.yml which should be committed
package.json	Adds preinstall hook to enforce Yarn usage and includes pnpm/yarn as devDependencies, but uses bash-only syntax that won't work on Windows
yarn.lock	Adds lockfile entries for [email protected] and [email protected] devDependencies
.github/workflows/main.yml	Adds repository cleanliness checks after test and package steps, but commands don't fail the build on errors

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This check will only print a message but won't fail the build. The command should exit with a non-zero status code to actually fail the CI job when the repository is dirty. Consider changing to:

if [[ -n $(git status --porcelain) ]]; then echo "repo is dirty, probably you missed to add some files to .gitignore"; exit 1; fi

Additionally, the check should run on all OS platforms (not just Linux), or use a shell: bash directive to ensure consistency. Currently, this will fail on Windows runners as they use PowerShell by default.

[Copilot]

This check will only print a message but won't fail the build. The command should exit with a non-zero status code to actually fail the CI job when the repository is dirty. Consider changing to:

if [[ -n $(git status --porcelain) ]]; then echo "repo is dirty, probably you missed to add some files to .gitignore"; exit 1; fi

Also, add a shell: bash directive to ensure this bash syntax works correctly.

Suggested change

	- run: if [[ -n $(git status --porcelain) ]]; then echo "repo is dirty, probably you missed to add some files to .gitignore"; fi
	- run: if [[ -n $(git status --porcelain) ]]; then echo "repo is dirty, probably you missed to add some files to .gitignore"; exit 1; fi
	shell: bash

[Copilot]

The preinstall script has a potential issue: it will fail on Windows systems because the grep command and bash-style scripting aren't available by default. This will prevent contributors on Windows from installing dependencies.

Consider using a cross-platform solution, such as:

1. A Node.js script instead of shell commands
2. The only-allow package which is specifically designed for this purpose
3. Adding a check for the OS before running the bash command

Example with Node.js:

"preinstall": "node -e \"if(process.env.npm_execpath.indexOf('yarn') === -1) { console.error('⚠️  Use yarn not npm! ⚠️'); process.exit(1); }\""

Suggested change

	"preinstall": "$npm_execpath --silent run checkyarn",
	"checkyarn": "echo \"$npm_execpath\" | grep -q \"yarn.*$\" || (echo '⚠️ Use yarn not npm! ⚠️ ' && echo && exit 1)",
	"preinstall": "node -e \"if(!process.env.npm_execpath || !/yarn/.test(process.env.npm_execpath)) { console.error('⚠️ Use yarn not npm! ⚠️'); process.exit(1); }\"",

[Copilot]

Adding pnpm as a devDependency is unconventional. Package managers are typically installed globally or used via their official distribution methods (e.g., Corepack). Including it as a devDependency adds unnecessary bloat to node_modules.

If you need pnpm for test fixtures (as seen in the pretest script), it's already available globally in the CI environment via pnpm/action-setup@v4. For local development, contributors should install it globally or use Corepack.

Suggested change

"pnpm": "^10.6.2",

[Copilot]

Adding yarn as a devDependency is unconventional and creates a circular dependency issue. Yarn is the package manager you're using to install dependencies, so it doesn't make sense to list it as a dependency to be installed by itself.

The project already has Yarn configured via .yarn/releases/yarn-classic.cjs. If you want to enforce a specific Yarn version, use the packageManager field in package.json (supported by Corepack):

"packageManager": "[email protected]"

This is the standard way to specify which package manager and version should be used.