Skip to content

Remove notes from allowed properties in label configuration schema #214

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

Merged
merged 3 commits into from
Mar 30, 2022
Merged

Remove notes from allowed properties in label configuration schema #214

merged 3 commits into from
Mar 30, 2022

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Mar 30, 2022

At the time the original configuration file was developed, it was in JSON, which does not support comments. I found the need to add some internal explanatory commentary to some of the labels, so I added an arbitrary notes key as a container for this information, and our JSON schema was also configured to accept this field.

I later decided to convert the files to YAML, since that is the language used by the majority of the asset configuration files. At this point it became possible to use comments, but the notes field was already in place so it seemed pointless to change it.

Validation of the configuration file was added to the "GitHub Label Sync" tool in the recent 2.1.0 release. Before 2.1.0, the tool ignored any additional properties in the label configuration objects. It now errors if there are any unexpected properties.

This notes field now causes the label configuration files that contain it to be considered invalid by the tool:

https://github.com/arduino/tooling-project-assets/runs/5754102563?check_suite_focus=true#step:9:13

Syncing labels for "arduino/tooling-project-assets"
Validating provided labels
Invalid label:
  {"name":"conclusion: moved","color":"940404","description":"Issue/PR moved to another repository","notes":"For use when it's not possible to do a true transfer\n(PRs, different organizations, or maintainer doesn't have write permissions in target repo).\n"}
  - must NOT have additional properties

[...]

So the notes field is hereby removed, with the contents moved to comments.


I have also updated the JSON schema used by this repository's CI system to validate the "shared" label configuration files hosted here, and by the "Sync Labels" template workflow to validate the local label configuration files. It is now aligned with the internal JSON schema of the "GitHub Label Sync" tool.

per1234 added 3 commits March 30, 2022 03:31
A JSON schema is used to validate the shared repository label configuration files hosted in this repository in order to ensure they are:

- Compliant with the restrictions of the GitHub labels API
- Compliant with the data format of the "GitHub Label Sync" tool used to manage the labels
- Consistent with the standardized labels format used in Arduino tooling projects

Validation of the configuration file was added to the "GitHub Label Sync" tool in the recent 2.1.0 release. Before
2.1.0, the tool ignored any additional properties in the label configuration objects. It now errors if there are any
unexpected properties.

At the time the original configuration file was developed, it was in JSON, which does not support comments. I found the
need to add some internal explanatory commentary to some of the labels, so I added an arbitrary "notes" key as a
container for this information, and our JSON schema was also configured to accept this field.

This "notes" field now causes the configuration files to be considered invalid by the tool, so our schema must do the
same.
A JSON schema is used to validate the shared repository label configuration files hosted in this repository in order to ensure they are:

- Compliant with the restrictions of the GitHub labels API
- Compliant with the data format of the "GitHub Label Sync" tool used to manage the labels
- Consistent with the standardized labels format used in Arduino tooling projects

The "GitHub Label Sync" tool supports an "aliases" array in the label configuration, which defines the labels which
should be renamed according to the configuration. This array is a list of label names, so the same fundamental
restrictions (both from GitHub and "GitHub Label Sync", but not Arduino's style guidelines) apply to it as to the `name`
field.
At the time the original configuration file was developed, it was in JSON, which does not support comments. I found the
need to add some internal explanatory commentary to some of the labels, so I added an arbitrary `notes` key as a
container for this information, and our JSON schema was also configured to accept this field.

I later decided to convert the files to YAML, since that is the language used by the majority of the asset configuration
files. At this point it became possible to use comments, but the `notes` field was already in place so it
seemed pointless to change it.

Validation of the configuration file was added to the "GitHub Label Sync" tool in the recent 2.1.0 release. Before
2.1.0, the tool ignored any additional properties in the label configuration objects. It now errors if there are any
unexpected properties.

This `notes` field now causes the configuration files that contain it to be considered invalid by the tool:

```
$ github-label-sync --labels https://raw.githubusercontent.com/arduino/tooling-project-assets/main/workflow-templates/assets/sync-labels/universal.yml per1234/some-repo
Syncing labels for "per1234/some-repo"
Validating provided labels
Invalid label:
  {"name":"conclusion: moved","color":"940404","description":"Issue/PR moved to another repository","notes":"For use when it's not possible to do a true transfer\n(PRs, different organizations, or maintainer doesn't have write permissions in target repo).\n"}
  - must NOT have additional properties

[...]
```

So the `notes` field is hereby removed, with the contents moved to comments.
@per1234 per1234 added type: imperfection Perceived defect in any part of project topic: code Related to content of the project itself labels Mar 30, 2022
@per1234 per1234 requested a review from umbynos March 30, 2022 12:46
@per1234 per1234 self-assigned this Mar 30, 2022
@per1234 per1234 merged commit fac9e8f into arduino:main Mar 30, 2022
@per1234 per1234 deleted the valid-label-configs branch March 30, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants