-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[MAINTENANCE] Remove URN support #9886
Conversation
✅ Deploy Preview for niobium-lead-7998 canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #9886 +/- ##
===========================================
- Coverage 77.99% 77.77% -0.23%
===========================================
Files 495 494 -1
Lines 42539 42399 -140
===========================================
- Hits 33178 32974 -204
- Misses 9361 9425 +64
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -413,30 +325,9 @@ def parse_suite_parameter( # noqa: C901, PLR0912, PLR0915 | |||
|
|||
elif len(parse_results) == 0 or parse_results[0] != "Parse Failure": | |||
# we have a stack to evaluate and there was no parse failure. | |||
# iterate through values and look for URNs pointing to a store: | |||
for i, ob in enumerate(EXPR.exprStack): |
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.
Is this a for/else
statement? Just confirming - is that intended or do we have a mis-indentation?
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.
Looks to me like this was not and is now not a for/else
loop. So pretty sure this is as intended, and no change in behavior other than not doing url stuff. The else
block looks to be matched in terms of logic with the elif
, and also I'd expect to see a break
in an for/else
, otherwise the else isn't doing anything special. Going to merge, but if any of that rings false to you, LMK and I'll revert!
Overview
This removes anything urn-related, which trickled down into deleting a couple more methods that weren't obvious from their names / that they were urn-related.
Summary of non-obvious methods that were deleted (Please double check me!)
_compile_suite_parameter_dependencies: used results of suite.get_suite_parameter_dependencies , which was just urn logic
invoke lint
(usesruff format
+ruff check
)For more information about contributing, see Contribute.
After you submit your PR, keep the page open and monitor the statuses of the various checks made by our continuous integration process at the bottom of the page. Please fix any issues that come up and reach out on Slack if you need help. Thanks for contributing!