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

generate row deserialization #68

Merged
merged 1 commit into from
Jun 10, 2024
Merged

generate row deserialization #68

merged 1 commit into from
Jun 10, 2024

Conversation

fcarreiro
Copy link

@fcarreiro fcarreiro commented Jun 7, 2024

Used for serialization to CSV.

@fcarreiro fcarreiro requested a review from Maddiaa0 June 7, 2024 17:35
Copy link
Collaborator

@jeanmon jeanmon left a comment

Choose a reason for hiding this comment

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

LGTM. Please have a look at my comments.

@@ -254,6 +258,60 @@ class {name}CircuitBuilder {{
&circuit_hpp,
);
}

fn create_circuit_builder_cpp(&mut self, name: &str, all_cols_with_shifts: &[String]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fcarreiro If this outputs also the "shift columns", I would rather make the inclusion of such columns optional. We will rarely need them. It will be rare that a bug emerges from a malformed shift column.

Copy link
Author

Choose a reason for hiding this comment

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

done!

@@ -6,7 +6,7 @@ authors = ["Maddiaa"]
edition = "2021"

[[bin]]
name = "bberg_pil"
name = "bb_pil"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am fine with the renaming but please announce this change in the Slack channel. I have an alias currently pointing to bberg_pil and I would continue to use an old version until it breaks. I do not think that cargo build would clean the old binary.

@fcarreiro fcarreiro merged commit c439294 into avm Jun 10, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants