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

Parsing of Pb_options according to protobuf schema #245

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

Lupus
Copy link
Contributor

@Lupus Lupus commented May 1, 2024

No description provided.

@Lupus Lupus force-pushed the generate-pb-option-parsers branch from 26af928 to c59e8e7 Compare May 1, 2024 14:58
@Lupus
Copy link
Contributor Author

Lupus commented Jun 5, 2024

@c-cube could you please take a look? This is required for further upstreaming of validation generator.

@c-cube
Copy link
Collaborator

c-cube commented Jun 5, 2024

I can look, but I've also been slowly trying to fix integration tests on master (which were silently broken). Would you look at #243 in the mean time?

@c-cube
Copy link
Collaborator

c-cube commented Jun 5, 2024

Right now, on my branch (and on this rebased on my branch) dune build src/tests/integration-tests/test_make.ml fails, and I bisected it back to either:

which might have broken packed repeated fields? :/

@Lupus
Copy link
Contributor Author

Lupus commented Jun 6, 2024

Right now, on my branch (and on this rebased on my branch) dune build src/tests/integration-tests/test_make.ml fails, and I bisected it back to either:

which might have broken packed repeated fields? :/

Yeah, something is wrong, I've got a repro, looking into this.

@c-cube
Copy link
Collaborator

c-cube commented Jun 7, 2024

merged #243 which conflicts with this, but we're close, I think :).

@Lupus Lupus force-pushed the generate-pb-option-parsers branch from cea4460 to d4e682b Compare June 11, 2024 06:23
@Lupus Lupus force-pushed the generate-pb-option-parsers branch from efd3f2c to 53bcc47 Compare June 11, 2024 06:57
@Lupus
Copy link
Contributor Author

Lupus commented Jun 11, 2024

Okay, I think I fixed it. This PR includes a small unrelated commit that forces compilation of option_processing.proto, and a fix to duplicate id fields in that file... I hope it's fine to have this extra commit on this PR :)

@c-cube
Copy link
Collaborator

c-cube commented Jun 11, 2024

It's great! Look at that, it even passes integration tests 😁 .

@c-cube c-cube merged commit 167505e into mransan:br-4.0 Jun 11, 2024
5 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