Skip to content

Backward compatibility when adding new languages #433

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
benalexau opened this issue Feb 8, 2017 · 2 comments
Closed

Backward compatibility when adding new languages #433

benalexau opened this issue Feb 8, 2017 · 2 comments

Comments

@benalexau
Copy link
Contributor

Commit 1e9a3e5 introduced error reporting if field names are reserved Golang keywords. This was subsequently included in the 1.5.6 release. Our many SBE projects now have build failures such as:

ERROR: at <sbe:message name="OHLC"> <field name="close"> name is not valid for Golang: close
ERROR: at <sbe:message name="Halt"> <field name="type"> name is not valid for Golang: type
ERROR: at <sbe:message name="OrderImbalance"> <field name="type"> name is not valid for Golang: type
...
Caused by: java.lang.IllegalStateException: had 3 errors
	at uk.co.real_logic.sbe.xml.ErrorHandler.checkIfShouldExit(ErrorHandler.java:105)
	at uk.co.real_logic.sbe.xml.XmlSchemaParser.parse(XmlSchemaParser.java:102)
	at uk.co.real_logic.sbe.SbeTool.parseSchema(SbeTool.java:268)
	at uk.co.real_logic.sbe.SbeTool.main(SbeTool.java:193)

This is even with sbe.validation.stop.on.error and sbe.validation.warnings.fatal both false, plus a sbe.keyword.append.token set (and I'd like to keep these warnings enabled anyway).

I'd suggest there are several related issues here:

  1. It seems undesirable that new languages should cause a warning (let alone an error) if a schema uses a reserved word. There are hundreds of keywords across hundreds of programming languages and rejecting existing schemas is particularly undesirable given SBE's focus as a long-term serialization format for inter-party communication.

  2. While SBE doesn't state anywhere that it observes Semantic Versioning, it's such a de facto standard these days that people commonly assume backward compatibility when only the patch field increments (major.minor.patch). It would be nice if SBE could help communicate to users there are breaking changes by incrementing the major and/or minor field (or perhaps officially adopt Semantic Versioning).

  3. The Change Log might be a good place to note that updating to 1.5.6 will reject schemas that use Go language keywords.

Fortunately our CI environment detected these issues immediately and we simply rolled back to 1.5.5. I offer the above feedback as constructive suggestions around future language support.

@mjpt777
Copy link
Contributor

mjpt777 commented Feb 8, 2017

Thanks @benalexau for such a good bug report. I think you are right that these should be a warning and not an error. I also think we should be stricter about semantic versioning. We strive for that but something make mistakes. I'll take the action to make these a warning.

@mjpt777
Copy link
Contributor

mjpt777 commented Mar 31, 2017

Future languages should use the TargetCodeGenerator interface implementation.

@mjpt777 mjpt777 closed this as completed Mar 31, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants