Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Let stable packages bypass LF version restrictions. #10377
Let stable packages bypass LF version restrictions. #10377
Changes from 14 commits
f1e6cb4
6dd741f
3f095f8
9c7c3a5
40ae433
ee3d4d9
fbf4e9e
60ccfcb
81071c2
643c1fb
f930922
fec1b49
1e5d575
c63576a
d4fda65
d6082c5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 (non-conformance) test that actually checks that (non-stable) packages older than 1.14 are rejected would be a nice to have.
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.
Hmm, there is already some tests for this at the engine level:
daml/daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineTest.scala
Line 2683 in 4b55f1a
But there's room for a test of the new flag
--daml-lf-min-version-1.14
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.
@mziolekda @miklos-da
This PR adds a feature needed by canton. However we need a way to test it through the full stack.
Are you fine we add this flag to be able to use the
ledger-on-memory
to test the feature ?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.
Yes, with the above small change. Though, it would be preferred to parse the actual version so that we don't have to modify this command-line parameter for the next Daml-LF version.
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.
I agree that it would be preferable to parse the actual min version, but note that there's no plan currently for the 1.14 minimum to change in the future, even when the next LF version is released.