-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Merge MetaMask/action-monorepo-release-pr #15
Conversation
73cbb3b
to
fdd7b26
Compare
* Implement action * Implement tests * Configure YAML files * Write README
The Jest options `resetMocks` and `restoreMocks` have been added to ensure all mocks are fully reset and restored between each test. This helps to prevent dependencies between tests, and makes some mistakes caused by misuse of shared mocks easier to spot (though it's not a perfect solution, and shared mocks should still be avoided if possible to further ensure complete test isolation).
Adds a check in CI via `git diff` to ensure that `yarn build` does not alter the contents of `dist`. The build files should always be updating before pushing to this repository.
* Update CI workflow name and triggers * Update workflow runs-on OS
* Refactor git-operations and its tests * Make execa mocks more sensible * Make sure all shared mocks are actually read-only
* Update readme to show how to enable hotfixes * Fix readme omission
This rule prevents the use of synchronous core library methods. There were no violations.
All TypeScript source files are now included in the coverage report. This includes files not touched by tests at all, which previously were omitted from the report. The coverage threshold was lowered to ensure the test script still passes.
These rules prevent accidentally forgetting to list production dependencies. They have been re-enabled. The only violation was caused by an import in a test, which was being flagged because all modules were considered as part of the final published package. There was no `.npmignore` file or `files` property in `package.json` to declare which files are published, so all files were assumed to be published. The `files` property has been added, so test files are no longer considered by these rules. Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
* Refactor for polyrepo compatibility * Restore coverage of package-operations * 100% test coverage of all files
* Add changelog updating * 100% test coverage * Use path.sep instead of "/" * Use path.join everywhere
fdd7b26
to
192350f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
}, | ||
"scripts": { | ||
"lint:eslint": "yarn eslint . --cache --ext js,ts", | ||
"lint:misc": "prettier '**/*.json' '**/*.md' '**/*.yml' --single-quote --ignore-path .gitignore", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a later PR, we should add .prettierrc.js
after this PR, and remove --single-quote
"lint:misc": "prettier '**/*.json' '**/*.md' '**/*.yml' --single-quote --ignore-path .gitignore", | ||
"lint": "yarn lint:eslint && yarn lint:misc --check", | ||
"lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write", | ||
"build:clean": "mkdir -p lib && rm -rf lib/* && mkdir -p dist && rm -rf dist/*", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should update this to use rimraf
in a subsequent PR
"build:clean": "mkdir -p lib && rm -rf lib/* && mkdir -p dist && rm -rf dist/*", | ||
"build:tsc": "tsc --project .", | ||
"build:ncc": "ncc build lib/index.js --out dist", | ||
"build": "yarn build:clean && yarn build:tsc && yarn build:ncc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a later PR, we should make build
a "non-clean" build, to allow incremental compilation. Maybe omit the build:ncc
for that too? 🤔 If it's just for local dev.
Also we should add a prepublishOnly
script to ensure the build runs.
Merges
MetaMask/action-monorepo-release-pr
into this repository. Should be merged outright, not squashed, in order to preserve the both git histories.action-monorepo-release-pr
was intended to beaction-create-release-pr
but for monorepos. Ultimately, it ended up implementing the same functionality as this Action, with monorepo compatibility. (Once you have monorepo compatibility, you effectively have "polyrepo" compatibility as well.)To minimize our maintenance burden, this PR merges the (slightly truncated) history of
action-monorepo-release-pr
into this repo. Everything but the TypeScript code takes a lot of inspiration from this repository and the experience of @shanejonas developing it, and preserving the history of the originalcreate-release-pr
Action is therefore apt, despite the fact that both git histories were originally separate.This PR preserves the changelog
.awk
script of this repository, but doesn't use it. A follow-up PR will have to either reincorporate that script when creating a message body for the release PR, adopt it for use with monorepos, or possibly just leave that toaction-publish-release
when it is refactored for monorepo-compatibility.