-
Notifications
You must be signed in to change notification settings - Fork 357
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
Enable more ruff rules #6057
base: main
Are you sure you want to change the base?
Enable more ruff rules #6057
Conversation
92c6c5c
to
3cde489
Compare
3cde489
to
dcd12bf
Compare
dcd12bf
to
6d89172
Compare
/kickstart-tests --testtype smoke |
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 good to me otherwise but I'm not happy about two changes which makes harder readability.
pyanaconda/ui/tui/hubs/__init__.py
Outdated
prompt.add_option("1", _("to enter the %(spoke_title)s spoke") % {"spoke_title": list(self._spokes.values())[0].title}) | ||
prompt.add_option( | ||
"1", | ||
_("to enter the %(spoke_title)s spoke") % {"spoke_title": next(iter(self._spokes.values())).title} |
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.
Honestly, this doesn't look like improvement. It's harder to read and understand.
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.
So keep this rule disabled ?
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 prefer that.
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 mean using next
is fine but it's harder to understand. At least for me.
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.
Ok removed this commit.
# we're looking for the original vg name | ||
idx = next((i for i, data in enumerate(self._containerStore) if data[0] == container_name), None) | ||
|
||
for idx, data in enumerate(self._containerStore): | ||
# we're looking for the original vg name | ||
if data[0] == container_name: | ||
break | ||
|
||
if idx: | ||
if idx is not None: |
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.
Again doesn't look to me like improvement.
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.
So keep this rule disabled ?
Or only this occurrence?
I dont have preference.
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, that would be my personal preference.
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.
Ok removed this commit.
6d89172
to
e93def2
Compare
* Replace 'dir' with 'directory' * Stop importing explicitely FileNotFound
Auto fix: * unnecessary-range-start (PIE808) * multiple-starts-ends-with (PIE810)
e93def2
to
5b6fb5a
Compare
No description provided.