Skip to content

Adapt protobuf test#611

Merged
thehoul merged 25 commits into
backward-compatible-marshallingfrom
adapt-protobuf-test
Feb 27, 2026
Merged

Adapt protobuf test#611
thehoul merged 25 commits into
backward-compatible-marshallingfrom
adapt-protobuf-test

Conversation

@thehoul

@thehoul thehoul commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

Depends on #608

Adapt protobuf test imported in internal to work in V4 and in the pipeline when compiling on 32b

@thehoul thehoul self-assigned this Feb 5, 2026
@github-actions

github-actions Bot commented Feb 5, 2026

Copy link
Copy Markdown

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

@thehoul
thehoul force-pushed the adapt-protobuf-test branch from d0891ba to 530031c Compare February 5, 2026 10:44
@github-actions

github-actions Bot commented Feb 5, 2026

Copy link
Copy Markdown

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

@thehoul thehoul added the safe PR label Feb 5, 2026
@thehoul
thehoul changed the base branch from master to move-protobuf-to-internal February 5, 2026 10:45
This was referenced Feb 5, 2026
@thehoul
thehoul marked this pull request as ready for review February 5, 2026 10:48
@thehoul
thehoul requested a review from jbsv February 5, 2026 10:48
@thehoul
thehoul requested a review from pierluca February 16, 2026 15:40

@jbsv jbsv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one question to be sure about the test.. Otherwise looks good to me 👍

Comment thread internal/protobuf/encoding_vartime_test.go Outdated

@AnomalRoil AnomalRoil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am a bit puzzled at the int removal parts.

Comment thread internal/protobuf/array_test.go
Comment thread internal/protobuf/crashers_test.go

type test struct {
Bool bool `protobuf:"boolean,opt"`
I int

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wait, are we not supporting encoding and decoding the int type?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The int type has no deterministic size (https://go.dev/ref/spec#Numeric_types), and we want to avoid that across architectures.

@sonarqubecloud

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@thehoul
thehoul force-pushed the move-protobuf-to-internal branch from 2f265e7 to fab66e0 Compare February 27, 2026 11:49
@thehoul
thehoul force-pushed the adapt-protobuf-test branch from 78c552f to 1743700 Compare February 27, 2026 11:50
@thehoul
thehoul changed the base branch from move-protobuf-to-internal to backward-compatible-marshalling February 27, 2026 12:04
@thehoul
thehoul merged commit bbf3d62 into backward-compatible-marshalling Feb 27, 2026
10 of 23 checks passed
@thehoul
thehoul deleted the adapt-protobuf-test branch February 27, 2026 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants