Skip to content

Conversation

@vkresch
Copy link
Contributor

@vkresch vkresch commented Jan 13, 2020

Reference to a related issue in the repository

#355

Add a description

This PR adds rules to each message field which can be parsed and used by the osi-validator to validate OSI trace files.

Some questions to ask:
What is this change?
This PR adds the feature to define rules in the comments which can be used by the validator. I also added a rules file called rules.yml in which the user can define the regex syntax for valid rules to be parsed.

How has it been tested?
It's been tested with the osi-validator which has a script which generates from the OSI comments yml rule files which then can be used by the validator.

Mention a member

@jdsika pls review and merge after the PR for the osi-validator. Add more rules if you think there should be a rule on a field.

Check the checklist

  • My code and comments follow the style guidelines and contributors guidelines of this project.
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests / travis ci pass locally with my changes.

@jdsika jdsika added this to the v3.1.3 milestone Jan 13, 2020
@jdsika jdsika added Documentation Everything which impacts the quality of the documentation and guidelines. Quality Quality improvements. labels Jan 13, 2020
@jdsika jdsika requested a review from pmai January 13, 2020 20:05
@jdsika
Copy link
Contributor

jdsika commented Jan 14, 2020

@pmai I would be very happy to get you opinion here!

The \endrules would be something I would like to get rid of. You are the language expert - whats your opinion. Deadline for merge 19.02 because of the release before the kickoff

Copy link
Contributor

@pmai pmai left a comment

Choose a reason for hiding this comment

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

Seems like a good way to go, I don't mind the doc-syntax.

It should be noted that for the range checking the default 0 values of fields when they are not filled should be taken into account in the actual checking (i.e. it is OK not to provide e.g. the barometric pressure, and in that case the default value will be 0, so that should be deemed valid, even if the range when a value is provided does not include 0); however that is something to handle in the os-validator itself; we should add documentation to the OSI doc however that explains the rules.

This commit adds the feature to define rules in the comments which
can be used by the validator. This adds a rules file called rules.yml
in which the user can define the regex syntax for valid rules to be
parsed.
@pmai
Copy link
Contributor

pmai commented Jan 27, 2020

Rebased against master, resolved conflicts, squashed to one commit; waiting for build.

@pmai pmai self-assigned this Jan 27, 2020
@pmai pmai merged commit c853a1c into master Jan 27, 2020
@vkresch vkresch mentioned this pull request Feb 17, 2020
6 tasks
@pmai pmai deleted the add-rules branch April 27, 2020 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Everything which impacts the quality of the documentation and guidelines. Quality Quality improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants