-
Notifications
You must be signed in to change notification settings - Fork 652
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
Adding ruff
rule A
#4232
base: main
Are you sure you want to change the base?
Adding ruff
rule A
#4232
Conversation
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.
Maybe we can just skip the A
instead?
docs/conf.py
Outdated
@@ -47,7 +47,7 @@ | |||
# -- Project information ----------------------------------------------------- | |||
|
|||
project = "OpenTelemetry Python" | |||
copyright = "OpenTelemetry Authors" # pylint: disable=redefined-builtin | |||
docs_copyright = "OpenTelemetry Authors" # pylint: disable=redefined-builtin |
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 am pretty sure we cannot change sphinx configuration for pleasing some lint rules 😅 see
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-copyright
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.
Can ignore this too
@@ -121,8 +121,8 @@ def _is_valid_value(value: object) -> bool: | |||
parts = str(value).split(";") | |||
is_valid_value = _VALUE_PATTERN.fullmatch(parts[0]) is not None | |||
if len(parts) > 1: # one or more properties metadata | |||
for property in parts[1:]: | |||
if _PROPERT_PATTERN.fullmatch(property) is None: | |||
for properties in parts[1:]: |
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.
Maybe prop
instead since this looks like is one property?
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.
prop
, properties
or no change?
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 would use prop
@@ -684,7 +684,7 @@ def start_span( | |||
context = SpanContextShim(span.get_span_context()) | |||
return SpanShim(self, context, span) | |||
|
|||
def inject(self, span_context, format: object, carrier: object): | |||
def inject(self, span_context, formats: object, carrier: object): |
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.
Not sure we can rename function parameters because a linter rule does not like it :)
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.
Yes, in this case, I think we can just ignore it.
Maybe I'll just revert back all changes and just stick to adding rules. :) |
Description
Address part of #4227
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
A
affects variable, argument, import, module names.)Does This PR Require a Contrib Repo Change?
Checklist: