-
Notifications
You must be signed in to change notification settings - Fork 13
Use op-cli-installer package #22
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
Conversation
…`op-cli-installer` now
If failed, it should not push had users have to fix formatting and eslint errors by himself and re-try to commit/push
edif2008
left a comment
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.
Code review: ✅
Straight forward and on point. It's larger due to the code being moved in the op-cli-installer package, which is reasonable. The other changes are also clear.
Functional review: ✅
The pipeline passes and I've also tested locally a simulated version of the workflow using act and it works as expected.
|
Will merge as soon as 1Password/op-cli-installer#1 is merged, to make sure we use latest commit hash here. |
This PR started to use "op-cli-installer" package and removes all the related source code from this repo.
Also it makes change in
lint-stagedconfiguration, so now lint-staged checks code formatting and lints it. If there are any errors it fails to commit/push and user should fix formatting and lint errors and try again.The unit tests are also removed from
lint-stagedshould be run as a part of ci/cd job. For now we removedRun jest testsfrom CI/CD as there are no tests in this repo, but all the underlying functionality is tested in op-cli-installer package.