Rule "No Removing Fields Without Reserve" Should not break while renaming an existing field about protolock HOT 9 CLOSED

Also, just noticed that you're using proto2 syntax, for which this tool doesn't claim to make any support guarantees. It probably works fine since the proto parser used supports the syntax and abstracts some of the differences (I think). I just wanted to point that out in case it becomes an issue in the future.

from protolock.

I'd have to double check, but you can disable this by disabling strict mode. Run protolock with the --strict=false and you should get the desired results.

What you describe is a breaking API change, however, as a client calling a generated method by the old name will no longer receive the value you intend them to unless they change their code.

It most likely won't cause any exceptions, but the tool still advises against the change without reserving the name in case you add this field back down the line and give it a different value than an existing client expects.

from protolock.

@nilslice I have tried it with --strict=false but still I get the same error because as per the documentation also --strict=false will not have an impact on this.

What you describe is a breaking API change, however, as a client calling a generated method by the old name will no longer receive the value you intend them to unless they change their code.

• As per my understanding client code will work fine because protobuf parser works on the tag number and not on the property name (I am keeping the tag number same), so old client code will still receive the value.

from protolock.

You're right - sorry, I'm on mobile only at the moment and misread the docs.

After dealing with large protobuf projects for a while, I have seen code break from a name-only change and find that this rule is important to follow. But if you think it should really be treated differently I would consider a PR, though would need to get feedback from my team and the other contributors who may already agree with the rule in place.

from protolock.

@cpeeyush - would adding this rule to be disabled when --strict is false make more sense to you?

I'm happy to do this, as I see your point regarding true "breaking" changes. I typically don't run --strict mode false. If you'd like to contribute it I would welcome the PR. Otherwise, I'm glad to make the change myself if we agree.

from protolock.

@cpeeyush - would adding this rule to be disabled when --strict is false make more sense to you?

I'm happy to do this, as I see your point regarding true "breaking" changes. I typically don't run --strict mode false. If you'd like to contribute it I would welcome the PR. Otherwise, I'm glad to make the change myself if we agree.

@nilslice Yes, I agree to disable this check with --strict false.
I would love to deep dive and submit a PR but right now I am not getting time to do that and honestly, I will take more time to modify your go lang code because it will be my first encounter with the go lang. So if you can do this change that would be awesome.

from protolock.

Ok, no problem. Thanks for opening the issue and making the suggestion.

Out of curiousity, what language(s) are you using protocol buffers with primarily?

from protolock.

Ok, no problem. Thanks for opening the issue and making the suggestion.

Out of curiousity, what language(s) are you using protocol buffers with primarily?

I am primarily using protocol buffers with java and trying to use your protolock library in my in-house schema registry to validate any schema change during the CI builds.

from protolock.

@cpeeyush - thanks for the response, that's good to know!

As I started to make the change for this, and re-read your initial issue, I realize I had misunderstood the situation you encountered, and the --strict mode set to false should still be your solution.

The reason is that your name change is okay to do, but you should reserve the old name so that it doesn't happen to provide a consumer with some new kind of data for a field that was previously being used for another kind of data.

The rule, NoChangingFieldNames, is the one that is toggled by --strict mode, and is the rule you probably want to allow changes.

In order to make this work, your local dev flow with protolock would be:

1. write your proto definition
2. change the name of a field (string name = 1 -> string full_name = 1)
3. reserve the field name name (reserved "name")
4. run protolock status --strict=false
5. observe no warnings reported
6. protolock commit --strict=false

I believe this will support the changes you're trying to make -- please let me know if that is not the case. The main concern here is that you still need to reserve the old field name after you change the field name to something new.

Closing this for now, but if this does in fact require some code changes, we can re-open.

from protolock.