-
Notifications
You must be signed in to change notification settings - Fork 44
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
Adopt schemaver #288
base: main
Are you sure you want to change the base?
Adopt schemaver #288
Conversation
@@ -27,7 +27,8 @@ | |||
|
|||
log = getLogger(__name__) | |||
SCHEMA_DIALECT = "http://json-schema.org/draft-07/schema#" | |||
SCHEMA_VERSION = "1" | |||
# We follow schemaver | |||
SCHEMA_VERSION = "1-1-0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we always use the latest version here, similar to what you're doing in docs/source/conf.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd run into a catch-22, because to bump it we'd need to first copy the latest file with a new name, and that feels clunky and hard to track via diffs and code reviews.
We could import from menuinst.platforms.base
though, but I didn't do that because (1) we would start depending on the project itself, and so far this module was standalone, and (2) this feels like the natural place to define the schema version. That's why I "tied" these hardcoded values via a test instead.
What do you think? Would you consider this a blocker?
@@ -22,6 +22,7 @@ | |||
) | |||
|
|||
log = getLogger(__name__) | |||
SCHEMA_VERSION = "1-1-0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, should we detect the latest version here instead of hard-coding it in two places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should here, because it's a runtime bit of the package. That code would require traversing a directory and so on "just" to avoid this known value. Also there might be scenarios where we have published a new schema preview or something, but we haven't migrated the runtime bits yet.
Description
Closes #287. Paired with conda/conda-build#5569.
Checklist - did you ...
news
directory (using the template) for the next release's release notes?