feat: create dedicated ecosystem name enum with all ecosystems #296
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
another-rex
approved these changes
Oct 20, 2024
| @@ -296,11 +296,55 @@ | |||
| "modified" | |||
| ], | |||
| "$defs": { | |||
| "ecosystem": { | |||
There was a problem hiding this comment.
I wonder if changing this ecosystem name will break anyone's workflow.
There was a problem hiding this comment.
I thought about that, but figured the likelihood anyone is depending on the name specifically right now is pretty low especially given no one seems to have caught how out of date this field actually is 😅
(the name should only matter if you're extending the schema in some way - if you're just using it for validating JSON files then at most it should result in a slightly different error message which shouldn't be depended on anyway)
|
I've opened #303 adding a script that handles automatically updating the enum and pattern |
Signed-off-by: Gareth Jones <jones258@gmail.com>
oliverchang
approved these changes
Oct 28, 2024
oliverchang
pushed a commit
that referenced
this pull request
Nov 5, 2024
This introduces a new script to make it easier to ensure all "lists of ecosystems" within this codebase remain up to date, including: - the table in `docs/schema.md` - the Go constants being introduced in #292 - the JSON schema in `validation/schema.json` (both the pattern and the enum being introduced in #296) To make it a bit easier, I've introduced a top-level `ecosystems.json` which is a map of defined ecosystems and a markdown description, sorted alphabetically (which the script also ensures) - I felt this was easier than trying to extract the list from markdown or another source, though it does mean double quotes need to be manually escaped. I went with JSON as it can be read without requiring an external dependency, though if we use Python 3.11 we could switch to `toml` instead as that ships with `tomllib` Example of the workflow output:   --------- Signed-off-by: Gareth Jones <jones258@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces new dedicated types for the name of an ecosystem (which is an enum) and an ecosystem suffix (which is a string starting with
:), along with renaming the existingecosystemtype toecosystemWithSuffix(+ adding it).While these types are not actually used in the schema due to JSONSchema currently does not have a way of specifying that a string is made up of two types, having them defined enables a lot of automation as the current
patternbased type cannot be used to get a finite list of all possible ecosystems defined by the spec.As an example, here's a TypeScript playground showcasing how you could use the types that would be automatically generated from this new schema structure using
json-schema-to-typescript(with a manually definedEcosystemSuffixtype, though we can later introduce that for TypeScript specifically by adding atsTypeproperty).Notably, landing this will unblock me adding automation to ensure that all lists of the ecosystem name are in sync (notably, the table in the docs, the enum + pattern here, and soon the constants being added in #292)