Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Script class defined twice #3332

Closed
kurtmckee opened this issue Jul 23, 2024 · 4 comments · Fixed by #3333
Closed

Script class defined twice #3332

kurtmckee opened this issue Jul 23, 2024 · 4 comments · Fixed by #3333

Comments

@kurtmckee
Copy link
Contributor

The Script class is defined twice in redis/commands/core.py, at lines 5462 and 6286:

redis-py/redis/commands/core.py

Line 5462 in 2ffcac3

class Script:

redis-py/redis/commands/core.py

Line 6286 in 2ffcac3

class Script:

git blame shows that both definitions were created on 2022-02-22, by different authors, within 1.5 hours of each other. I intend to submit a PR to delete the first definition, as the first definition is overwritten by the second definition as the file is parsed.
kurtmckee added a commit to kurtmckee/pr-redis-py that referenced this issue Jul 23, 2024
@kurtmckee
Delete the first-defined (and thus "duplicate") Script class
d21bce6 
The type annotations are copied to the second definition, and
the mutable default arguments to the `keys` and `args` parameters
are replaced with `None`, as is best-practice.

Closes redis#3332
kurtmckee added a commit to kurtmckee/pr-redis-py that referenced this issue Jul 23, 2024
@kurtmckee
Delete the first-defined (and thus "duplicate") Script class
cf7583a 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes redis#3332
kurtmckee added a commit to kurtmckee/pr-redis-py that referenced this issue Jul 23, 2024
@kurtmckee
Delete the first-defined (and thus "duplicate") Script class
77ddb9b 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes redis#3332
@kurtmckee kurtmckee mentioned this issue Jul 23, 2024
Merged
6 tasks
kurtmckee added a commit to kurtmckee/pr-redis-py that referenced this issue Jul 31, 2024
@kurtmckee
Delete the first-defined (and thus "duplicate") Script class
cb4d8a7 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes redis#3332
vladvildanov added a commit that referenced this issue Sep 2, 2024
@kurtmckee @vladvildanov
Delete the first-defined (and thus "duplicate") Script class (#3333)
93b9d85 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes #3332

Co-authored-by: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com>
@akx
Copy link
Contributor

akx commented Sep 24, 2024

This would be caught by a linter such as Ruff: #3147

https://docs.astral.sh/ruff/rules/redefined-while-unused/

@kurtmckee
Copy link
Contributor Author

But then you would be using ruff.

@akx
Copy link
Contributor

akx commented Sep 25, 2024

But then you would be using ruff.

This is true. Is that a problem?

@kurtmckee
Copy link
Contributor Author

I apologize, my comment just created disharmony and didn't contribute anything useful. Please disregard.

vladvildanov added a commit that referenced this issue Sep 27, 2024
@kurtmckee @vladvildanov
Delete the first-defined (and thus "duplicate") Script class (#3333)
ea5c957 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes #3332

Co-authored-by: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com>
vladvildanov added a commit that referenced this issue Sep 27, 2024
@kurtmckee @vladvildanov
Delete the first-defined (and thus "duplicate") Script class (#3333)
16a3606 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes #3332

Co-authored-by: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com>
vladvildanov added a commit that referenced this issue Sep 27, 2024
@kurtmckee @vladvildanov
Delete the first-defined (and thus "duplicate") Script class (#3333)
9933692 
The second definition was copied over the first definition,
with the following changes:

* The type annotations were copied to the second definition
* The mutable default arguments to the `keys` and `args` parameters
  were replaced with `None`, as is best-practice.

Closes #3332

Co-authored-by: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Delete the first-defined (and thus "duplicate") Script class kurtmckee/pr-redis-py
2 participants
@kurtmckee @akx