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

Set schema for manifest #2099

Merged
merged 8 commits into from May 7, 2024
Merged

Conversation

ppfeister
Copy link
Collaborator

@ppfeister ppfeister commented May 6, 2024

Nothing major here.

Sets the schema for data[.]json to allow for autocomplete, validation, etc. Reduces the likelihood of things being missed or left behind.

Setting the schema happened to uncover a couple minor instances of this (old username_unclaimed, noPeriod, etc). Those have been cleared.

I may adapt this in the future to disallow keys such as errorMsg when they don't match their dependent errorTypes, but that's a later problem. Done.


Looks like this was also mentioned back in #1336 and #1338. (so, closes #1336, closes #1338, if merged)

sherlock/resources/data.schema.json Outdated Show resolved Hide resolved
Copy link
Member

@sdushantha sdushantha left a comment

Choose a reason for hiding this comment

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

Your changes somehow added the following in the data.json. But I can't seem to find this text in your commit on GitHub. Got any idea on what happend?

WindowsTerminal_jgiqGKEpV0

@ppfeister
Copy link
Collaborator Author

ln980 was added in cc95d4e and ln2268 in 1b14b88, both of #2068.

The errorMsg str lists of those two items are somewhat vague and don't always have an impact, and it's possible that someone would remove one thinking that it's no longer needed.

Sorta like how YouTube gave us completely different results depending on region --- something that not everyone would pick up on in a quick test

@ppfeister
Copy link
Collaborator Author

huh. now that you mention it, those two lines are not compliant with the schema...

time to update

@ppfeister
Copy link
Collaborator Author

Updated so that mixed error values are flagged.

i.e. Leaving an errorCode present while errorType is set to message would be yellow-lined.

@sdushantha sdushantha merged commit d0c8282 into sherlock-project:master May 7, 2024
2 checks passed
@ppfeister ppfeister mentioned this pull request May 8, 2024
1 task
@ppfeister ppfeister deleted the feature/schema branch May 8, 2024 02:44
sdushantha added a commit that referenced this pull request May 9, 2024
Bumping version due to #2106 and #2099
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define JSON Schemas to put at the top of each JSON file
2 participants