-
Notifications
You must be signed in to change notification settings - Fork 5
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
Feature/compiler version #37
Conversation
Hrom131
commented
Oct 10, 2024
•
edited
Loading
edited
- Since this PR suggests a bug fix, the relevant tests have been added.
- Since this PR introduces a new feature, the update has been discussed in an Issue or with the team.
- This PR is just a minor change, like a typo fix.
`${OLDEST_SUPPORTED_CIRCOM_VERSION} - ${LATEST_SUPPORTED_CIRCOM_VERSION}`, | ||
); | ||
|
||
if (isVersionStrict && !semver.satisfies(version, supportedVersionsRange!)) { |
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.
Why only when the version is strict?
Also there are js comments that need to be fixed.
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 may have a case where there is a version in the circuit, for example 2.0.0 and no version is set in the config. In this case we don't have a strict version and we will take the maximum supported version. Without this check, we will get an error that the version is not supported and should be within 2.0.5 - 2.1.9.
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.
But what if the circuit has pragma 2.2.0
inside? Currently this version is not supported
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!