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

warn if XSD version does not match build version in validate command (DAT-9874) #3016

Merged
merged 5 commits into from Jul 11, 2022

Conversation

StevenMassaro
Copy link
Contributor

@StevenMassaro StevenMassaro commented Jun 28, 2022

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Warn if specified XSD version is different than the version of the build when running the validate command.

Things to be aware of

I made this change in a way that this check is "best effort". It should not ever cause a user to experience an uncaught exception.

Things to worry about

It is possible that the regex I'm using is not robust enough to properly get the version out of the XSD URL. It is also possible that there is a better way to do this than using a regex.

Additional Context

@StevenMassaro StevenMassaro changed the title warn user if XSD version does not match build version (DAT-9874) warn if XSD version does not match build version (DAT-9874) Jun 28, 2022
@github-actions
Copy link

github-actions bot commented Jun 28, 2022

Unit Test Results

  4 548 files  ±    0    4 548 suites  ±0   34m 35s ⏱️ + 2m 4s
  4 541 tests +  10    4 327 ✔️ +  10     214 💤 ±0  0 ±0 
53 784 runs  +120  48 776 ✔️ +120  5 008 💤 ±0  0 ±0 

Results for commit 26f322c. ± Comparison against base commit 17ac81c.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

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

I’m not sure this is a good idea?

People usually have a large set of “previous” change logs that have built up over the years. Like a v1.0.xml and v1.1.xml … v8.3.xml

All those old versions were working fine, including the xsd they are referencing. Ideally as you move on to new changelog files you shouldn't be updating those old files, but this change will bother them until they do.

I get that people can have issues if they don't notice that they are on an old xsd, but we are also now suggesting they don't use a versioned xsd and instead use -latest unless they have a good reason. Under that best practice, this warning makes even less sense because they have picked a specific version for a reason and so even less of a “warning “ ?

@kataggart kataggart added this to To Do in Conditioning++ via automation Jun 29, 2022
@kataggart kataggart added the XSD label Jun 29, 2022
@StevenMassaro
Copy link
Contributor Author

I’m not sure this is a good idea?

People usually have a large set of “previous” change logs that have built up over the years. Like a v1.0.xml and v1.1.xml … v8.3.xml

All those old versions were working fine, including the xsd they are referencing. Ideally as you move on to new changelog files you shouldn't be updating those old files, but this change will bother them until they do.

I get that people can have issues if they don't notice that they are on an old xsd, but we are also now suggesting they don't use a versioned xsd and instead use -latest unless they have a good reason. Under that best practice, this warning makes even less sense because they have picked a specific version for a reason and so even less of a “warning “ ?

We're going to display this message only when running the validate command, and display it at an info level.

@StevenMassaro StevenMassaro changed the title warn if XSD version does not match build version (DAT-9874) warn if XSD version does not match build version in validate command (DAT-9874) Jun 30, 2022
@StevenMassaro StevenMassaro merged commit 896ca4b into master Jul 11, 2022
Conditioning++ automation moved this from To Do to Done Jul 11, 2022
@StevenMassaro StevenMassaro deleted the DAT-9874 branch July 11, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants