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

FIELD_SAME_ONEOF is not triggering #2886

Closed
MollardMichael opened this issue Apr 10, 2024 · 5 comments
Closed

FIELD_SAME_ONEOF is not triggering #2886

MollardMichael opened this issue Apr 10, 2024 · 5 comments

Comments

@MollardMichael
Copy link

I might have missunderstood the usage of FIELD_SAME_ONEOF as a breaking rule.

My understanding was that it would trigger as long as I add or remove a new field in a oneof .

So I tried the following

     oneof statusInfo {
         Booked booked = 100;
+        Booked booked_2 = 101;
     }

Using buf version 1.30.1.

I was expecting it to trigger the rule but it is not.
Am I doing something wrong?

My buf.yaml

version: v1
lint:
  use:
    - DEFAULT
breaking:
  use:
    - WIRE
@emcfarlane
Copy link
Contributor

FIELD_SAME_ONEOF will trigger when the same tag is moved out (or in) of the oneof. So:

     oneof statusInfo {
         Booked booked = 100;
-       Booked booked_2 = 101;
     }
+   Booked booked_2 = 101;

See the details on oneof rules: proto3

@MollardMichael
Copy link
Author

Is there anyway to detect changes on oneof?

It's can be quite breaking

@emcfarlane
Copy link
Contributor

Adding fields is not a breaking change and part of the benefits. Maybe you could explain what you are hoping to achieve or what issues you are having?

@MollardMichael
Copy link
Author

While it's not breaking the message itself, it make the consumer unable to understand it. (Consumer needs to be updated before the producer can use this new field). I'd like to be able to be warned about that one way or another I guess

@bufdev
Copy link
Member

bufdev commented May 18, 2024

Adding a new field to a oneof isn't something we would consider breaking, and the buf CLI takes a strict stance that it only will print errors from buf lint or buf breaking, and won't consider warnings of advisories. Users need to know that if buf lint or buf breaking prints an error, it really is an error, and it can't be ignored base on their configuration. Warnings result in low signal-to-noise ratio that result in them just being ignored. Going to close this issue, but thanks for the feedback.

@bufdev bufdev closed this as completed May 18, 2024
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

No branches or pull requests

3 participants