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.

Sign up for GitHub

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

feat(json-schema-check): add new action #240

Merged
merged 3 commits into from
Jun 26, 2023
+45 −0
Merged

feat(json-schema-check): add new action #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f938d27
feat(json-schema-check): add new action
ambroise-arm Jun 20, 2023
e7375e1
Update json-schema-check/README.md
kenji-miyake Jun 26, 2023
eb12321
Update README.md
kenji-miyake Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ See the `README.md` in each directory.
- [generate-changelog](./generate-changelog/README.md)
- [get-modified-packages](./get-modified-packages/README.md)
- [get-self-packages](./get-self-packages/README.md)
- [json-schema-check](./json-schema-check/README.md)
- [pre-commit](./pre-commit/README.md)
- [register-autonomoustuff-repository](./register-autonomoustuff-repository/README.md)
- [remove-exec-depend](./remove-exec-depend/README.md)
Expand Down
29 changes: 29 additions & 0 deletions json-schema-check/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# json-schema-check

## Description

This action checks if the ROS 2 parameter files (`config/*.param.yaml`) of packages comply with the format of their template JSON Schema file (`schema/*.schema.json`).

## Usage

```yaml
jobs:
json-schema-check:
runs-on: ubuntu-latest
steps:
- name: Check out repository
uses: actions/checkout@v3
with:
fetch-depth: 0

- name: Run json-schema-check
uses: autowarefoundation/autoware-github-actions/json-schema-check@v1
```
## Inputs
None.
## Outputs
None.
15 changes: 15 additions & 0 deletions json-schema-check/action.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: json-schema-check
description: ""

runs:
using: composite
steps:
- name: Install JSON Schema packages
run: |
pip3 install -U check-jsonschema
shell: bash

- name: Check configuration files
run: |
find -wholename '*/schema/*.schema.json' -execdir bash -c 'check-jsonschema --schemafile "$1" ../config/*.param.yaml' bash '{}' +
Copy link
Contributor

Choose a reason for hiding this comment

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

1 schema/{pkg_name}.schema.json that acts the template for the configuration files
1 (or more) config/*.param.yaml configuration that need to match the template

It seems these behaviors aren't implemented yet.
Is this pull request still under draft or do you plan to implement them in another pull request?
If it's the former, please mark this pull request as a draft.
If it's the latter, creating an issue would be good.
(Or am I misunderstanding something?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The design for the structure has been published here: https://github.com/orgs/autowarefoundation/discussions/3371
It has been implemented as an example for lidar_apollo_segmentation_tvm_nodes. A second one was added recently with autoware_auto_msgs_adapter. The rest is still to be done.
The idea is that in order for people to add those files, it is valuable to have the format checked in CI before they start to create patches. Otherwise we risk having incorrect files in the repositories and it will take time later to fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I misunderstood one thing. The correspondence between param.yaml and schema.json was checked in the command.

@ambroise-arm So do you want to merge this as it is?
I mean, it seems that this action succeeds if a package doesn't have any schema.json. Is that okay for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update the spelling with ROS 2, otherwise yes.
Should I update .github/workflows/test-composite-actions.yaml and the main README to add this? Or anything else that I missed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should I update .github/workflows/test-composite-actions.yaml

It's not mandatory, but it's preferable if you can test this action.

the main README to add this?

Ah, yes. It would be nice.

Or anything else that I missed?

No, looks good! Thank you for your work.

I'll make a new release after merging it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've fixed them, so I'll merge this.

shell: bash