Skip to content
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

Rollup enable by default #143

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Rollup enable by default #143

merged 1 commit into from
Oct 30, 2023

Conversation

vfusco
Copy link
Contributor

@vfusco vfusco commented Oct 10, 2023

closes #102

@vfusco vfusco added the enhancement New feature or request label Oct 10, 2023
@vfusco vfusco added this to the v0.16.0 milestone Oct 10, 2023
@vfusco vfusco self-assigned this Oct 10, 2023
src/cartesi-machine.lua Show resolved Hide resolved
src/cartesi-machine.lua Show resolved Hide resolved
@@ -448,7 +446,13 @@ local init_splash = true
local append_bootargs = ""
local append_init = ""
local append_entrypoint = ""
local rollup
local rollup = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only solves for cartesi-machine.lua, what about the C API cm_get_default_config ? Shoudn't we also make this the default there? cc @diegonehab

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It should be default in C++ as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not get the default config in cartesi-machine.lua, right? We build one from scracth. Should we stop doing that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Victor will create an issue about 'get_default_config' vs. 'cartesi-machine.lua'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vfusco vfusco force-pushed the feature/rollup-default branch 3 times, most recently from e4e0ec5 to 0bca727 Compare October 27, 2023 21:22
@vfusco vfusco requested a review from mpolitzer October 27, 2023 21:22
src/machine-config.h Outdated Show resolved Hide resolved
@vfusco vfusco merged commit f1c9860 into main Oct 30, 2023
7 checks passed
@vfusco vfusco deleted the feature/rollup-default branch October 30, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Have rollup enabled by default
4 participants