feat!: validate Avro schemas included in AsyncAPI docs #96
Conversation
|
@dalelane I see a remark about breaking change. Basically, we are afraid that enabling validation can cause failing parsing for others? the Avro parser is already used by many, and in production, and is in a good shape. We can release 1.0 imho. So even if we are not 100% sure if it is a breaking feature, we can play safe thoughts? |
yeah, I think that would be good To be fair, I think the risk is very small - because if someone really did have an invalid Avro schema in this way, most other Avro tools/clients/libraries I'm aware of would've already rejected it. But, there is a small chance that someone has a slightly-invalid Avro schema in an AsyncAPI doc that today would parse correctly - and upgrading the parser would cause it to no longer parse. The Avro schemas we had in the tests are examples of how that can happen, as I needed to fix them! :-) |
|
Normally we do not treat these things as breaking changes. For example, the main JS Parser doesn't have 100% validation coverage, and when we add new ones, like tags validation and at some point of time examples validation, we do not make a major release. But yeah, here we are in this good position that we can actually do a major if we all think it is ok. Count on my vote |
There was a problem hiding this comment.
@dalelane it could be nice to add a new section in the Features chapter in the README file to indicate that we now provide avro validation during the processing ?
Furthermore, if we decide to release as a breaking change version, don't forget to reword your rebased commit with the ! char, according to the conventionalcommits.org specification :)
|
yes, to release major, title must be |
This commit adds validation to the avro-schema-parser so that invalid Avro schemas will be rejected. I had to fix a few invalid Avro schemas in the tests (which were using the type 'uuid' which isn't in the Avro spec) so this should potentially be considered a major breaking change to the parser. Closes: #37 Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
dalelane
force-pushed
the
avro-validation
branch
from
e511cc8
to
b2b6214
Compare
dalelane
changed the title
|
added an explanation to README, and squashed-and-rebased |
Yep it seems ok for me :) |
|
@derberg brill, thanks - I've squashed and rebased, updated the commit title, and made the README change - think I just need a re-review please then we're good to go |
|
@dalelane oh, sorry for misunderstanding, I just meant that when you will click button anyway, tests hang there in the system and won't run. Could you push an empty commit maybe to trigger checks please |
Signed-off-by: Dale Lane <dale.lane@uk.ibm.com>
|
Kudos, SonarCloud Quality Gate passed!
|
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Congrats! please only remember folks that even though you are committers you should work on forks like every other contributor, not create branches in the upstream. I recommend this instruction to set up your fork to work with upstream seamlessly |
|
ah, I didn't realise - thanks for the heads-up, will do that next time |
This commit adds validation to the avro-schema-parser so that
invalid Avro schemas will be rejected.
I had to fix a few invalid Avro schemas in the tests (which were
using the type 'uuid' which isn't in the Avro spec) so this should
potentially be considered a major breaking change to the parser.
Closes: #37
Signed-off-by: Dale Lane dale.lane@uk.ibm.com