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

Update result file schema and add demo pipeline #86

Merged
merged 14 commits into from
Jun 20, 2024
Merged

Update result file schema and add demo pipeline #86

merged 14 commits into from
Jun 20, 2024

Conversation

hoangtungdinh
Copy link
Collaborator

@hoangtungdinh hoangtungdinh commented Jun 18, 2024

Description

This PR is ready for CCB's review.

This PR updates the result file schema, taking into account all the requirements. All related C++ classes are also updated to reflect the new schema.

This PR also adds a docker-based demo pipeline that supports the checker bundles for OpenDrive and OpenScenario.

Main changes

  1. Update the result file schema and the corresponding C++ classes. The new result file schema addresses all the requirements and is expected to be stable.
  2. Update the text report module to report the list of addressed rules and violated rules from the result files.
  3. Add the demo pipeline. The demo pipeline is available on the GitHub Container Registry and will be updated every night at 4 am UTC.
  4. Some minor improvements

How was the PR tested?

  1. Tests are documented in each PR above.

Notes
This PR closes the following issues

Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>
@hoangtungdinh hoangtungdinh added the isState:ForCCBReview CCB will review it and change the status to ReadyForMerge if everything is ok label Jun 18, 2024
Signed-off-by: hoangtungdinh <[email protected]>

Signed-off-by: hoangtungdinh <[email protected]>
romanodanilo and others added 2 commits June 19, 2024 13:17
Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: romanodanilo <[email protected]>

Signed-off-by: hoangtungdinh <[email protected]>

Signed-off-by: hoangtungdinh <[email protected]>
Copy link
Collaborator

@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.

Overall the changes look good to me.

The minor point I'm raising on the XML/Schema front is for architectural discussion: We should likely tighten down the schema quite a bit more, and be careful about the semantics of attribute values: Usually you want to distinguish a missing value from an empty string/value. But again this is for future direction and PRs, I think.


<CheckerBundle build_date="" description="" name="DemoCheckerBundle" summary="Found 3 issues" version="">
<Checker checkerId="exampleChecker" description="This is a description" status="completed" summary="">
<Issue description="This is an information from the demo usecase" issueId="0" level="3" ruleUID=""/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should a non-exisiting ruleUID not be handled via eliding the attribute rather than supplying an empty string?

More generally I think going forward the schema should be tightened down to specify required attributes (currently all are optional) and more specific types for the attributes, where useful (which would e.g. disallow empty strings in places where IDs are required). This is less of a comment on the current PR, but again more for the forward-looking architectural discussions.

@pmai
Copy link
Collaborator

pmai commented Jun 20, 2024

@AsamDiegoSanchez @hoangtungdinh same here: was this officially approved in the CCB? Then we should tag as ready for merge, and I can perform the merge.

@AsamDiegoSanchez AsamDiegoSanchez added isState:ReadyForMerge An issue that has been integrated and is assigned to the workflow manager and removed isState:ForCCBReview CCB will review it and change the status to ReadyForMerge if everything is ok labels Jun 20, 2024
@AsamDiegoSanchez
Copy link
Member

yes, it was approved without any further comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants