-
Notifications
You must be signed in to change notification settings - Fork 515
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
[berkeley] Remove stop-slots from compile config #15589
base: develop
Are you sure you want to change the base?
[berkeley] Remove stop-slots from compile config #15589
Conversation
!ci-build-me |
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 wonder why infra reviewers are called for this PR. Seems a lot of protocol to me :)
@@ -67,7 +67,7 @@ let max_event_elements = 100 | |||
|
|||
let max_action_elements = 100 | |||
|
|||
[%%inject "network_id", network] | |||
let network_id = "testnet" |
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.
Wouldn't this impact when building from source using mainnet profile?
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, but only locally. In CI, this is still true. It's also only used in one janky GraphQL endpoint anyway, where it's overridden by the ledger.name
in the runtime config.
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.
It seems like there are members in the community that build from source and not use the deb or docker images from releases. So we'll need to keep that option available
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.
It's also only used in one janky GraphQL endpoint anyway, where it's overridden by the ledger.name in the runtime config.
oh :/
Is this change necessary in It looks safe and sane, but I'd prefer to avoid any changes to As for these changes, I suggest we merge them to |
This PR removes the stop slots from the compile config.
These caused a previous issue where a release was baked expecting it to use the compile config, where CI is not set up to do so. Instead, we remove the options so that they are only injected in the correct location: in the runtime config that we provide for the network.