Skip to content

Remove notes field from label configuration file #699

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 1 commit into from
Mar 31, 2022
Merged

Remove notes field from label configuration file #699

merged 1 commit into from
Mar 31, 2022

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Mar 31, 2022

At the time the reference 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, and by our custom JSON schema:

https://github.com/arduino/arduino-create-agent/runs/5767351577?check_suite_focus=true#step:5:72

.github/label-configuration-files/labels.yml invalid
[
  {
    instancePath: '/0',
    schemaPath: '#/items/additionalProperties',
    keyword: 'additionalProperties',
    params: { additionalProperty: 'notes' },
    message: 'must NOT have additional properties'
  },
  {
    instancePath: '/1',
    schemaPath: '#/items/additionalProperties',
    keyword: 'additionalProperties',
    params: { additionalProperty: 'notes' },
    message: 'must NOT have additional properties'
  }
]

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

At the time the reference 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, and by our
custom JSON schema:

```
.github/label-configuration-files/labels.yml invalid
[
  {
    instancePath: '/0',
    schemaPath: '#/items/additionalProperties',
    keyword: 'additionalProperties',
    params: { additionalProperty: 'notes' },
    message: 'must NOT have additional properties'
  },
  {
    instancePath: '/1',
    schemaPath: '#/items/additionalProperties',
    keyword: 'additionalProperties',
    params: { additionalProperty: 'notes' },
    message: 'must NOT have additional properties'
  }
]
```

So the `notes` field is hereby removed, with the contents moved to comments.
@per1234 per1234 added topic: infrastructure Related to project infrastructure type: imperfection Perceived defect in any part of project labels Mar 31, 2022
@per1234 per1234 requested a review from umbynos March 31, 2022 08:20
@per1234 per1234 self-assigned this Mar 31, 2022
@per1234 per1234 merged commit c3abaac into arduino:main Mar 31, 2022
@per1234 per1234 deleted the valid-labels branch March 31, 2022 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: infrastructure Related to project infrastructure type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants