-
Notifications
You must be signed in to change notification settings - Fork 79
Add synapse_requirement to storm package schema (SYN-5993) #3304
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
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3304 +/- ##
==========================================
- Coverage 97.26% 97.16% -0.11%
==========================================
Files 228 228
Lines 45675 45679 +4
==========================================
- Hits 44426 44384 -42
- Misses 1249 1295 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
synapse/tests/test_lib_ast.py
Outdated
@@ -20,7 +20,8 @@ | |||
'name': 'foo', | |||
'desc': 'The Foo Module', | |||
'version': (0, 0, 1), | |||
'synapse_minversion': (2, 8, 0), | |||
'synapse_minversion': [2, 144, 0], | |||
'synapse_version': '>=2.8.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 these be ">=2.8.0,<3.0.0" since people are likely to copy them? Should we also remove the synapse_minversion
keys from any tests which are potential examples?
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.
Yeah I'll add <3.0.0
to these, keeping a synapse_minversion
in there makes sure that the cortex is updated enough to actually check the synapse_version
requirement.
No description provided.