-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor to use new common packages + remove eik map command #511
Conversation
BREAKING CHANGE: the map command has been removed in favour of a single publish command
it should only use the configuration version to avoid version mismatch
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.
Looks good!
Co-authored-by: Richard Walker <digitalsadhu@gmail.com>
classes/integrity.js
Outdated
|
||
const { ValidationError } = schemas; |
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.
A little bit of consistency nit picking here, but I find the deconstruction here a little bit inconsistent.
This means that some places we are doing ex so:
schemas.assert.version(this.version);
and other places so:
throw new ValidationError(`Parameter "debug" is not valid`);
I would prefer that we either deconstructed both so we do as follow:
assert.version(this.version);
throw new ValidationError(`Parameter "debug" is not valid`);
or non so we do as follow:
schemas.assert.version(this.version);
throw new schemas.ValidationError(`Parameter "debug" is not valid`);
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.
Looks good to me if we fix my nit picking 👍
🎉 This PR is included in version 3.0.0-next.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR implements the previous
eik map
command intoeik publish
, as well as moving over to the new separated common packages.