Skip to content

Allow omitting the standard enum prefix#59

Open
Alfus wants to merge 8 commits intomainfrom
alfus/enum
Open

Allow omitting the standard enum prefix#59
Alfus wants to merge 8 commits intomainfrom
alfus/enum

Conversation

@Alfus
Copy link
Contributor

@Alfus Alfus commented Apr 24, 2025

When StrictEnums is false, the following is now allowed:

  - oneof_enum_value: VALUE_1

The following is always allowed:

  - oneof_enum_value: PROTO2_TEST_ENUM_VALUE_1

Note that this does not impact 'encoding', which always produces protojson-compatible output.

@Alfus Alfus requested a review from pkwarren April 24, 2025 20:39
Copy link
Member

@bufdev bufdev left a comment

Choose a reason for hiding this comment

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

Non-strict should definitely not be the default. This makes protoyamls behavior different than protojson, and overall surprising.

Copy link
Member

@pkwarren pkwarren left a comment

Choose a reason for hiding this comment

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

Change looks good but agree this shouldn't be the default.

Do you plan to also add a plugin option in the JSON schema plugin to include shorter enum names?

@Alfus
Copy link
Contributor Author

Alfus commented Apr 25, 2025

Every change also requires a change to the json schema plugin.
Protoyaml input has never been (and never needs to be) valid protojson. Only the output is normalized to be compatible.

@Alfus
Copy link
Contributor Author

Alfus commented Apr 25, 2025

Honestly, because there are no backwards compatibility issues, there should be no option. This change only changes what used to be an error in to an intuitive non-error case. If any option should be supported it is the option to specify which one you want the jsonschema plugin to suggest, which is external to this library.

Alfus and others added 4 commits April 25, 2025 08:16
Co-authored-by: Philip K. Warren <pkwarren@users.noreply.github.com>
Co-authored-by: Philip K. Warren <pkwarren@users.noreply.github.com>
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.

3 participants