Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Delete the first-defined (and thus "duplicate") Script class #3333

Merged

Conversation

kurtmckee
Copy link
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

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

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 force-pushed the rm-duplicate-Script-class-issue-3332 branch from 77ddb9b to cb4d8a7 Compare July 31, 2024 12:19
@kurtmckee
Copy link
Contributor Author

Rebased on master

@kurtmckee
Copy link
Contributor Author

Hello @gerzse and @vladvildanov! Please review this PR when you can. It removes a duplicate class definition and is part of my effort to get CI tests to pass in this repo.

@vladvildanov vladvildanov added breakingchange API or Breaking Change and removed breakingchange API or Breaking Change labels Sep 2, 2024
@vladvildanov vladvildanov merged commit 93b9d85 into redis:master Sep 2, 2024
56 checks passed
@kurtmckee kurtmckee deleted the rm-duplicate-Script-class-issue-3332 branch September 2, 2024 12:41
vladvildanov added a commit that referenced this pull request Sep 27, 2024
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 pull request Sep 27, 2024
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 pull request Sep 27, 2024
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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Script class defined twice
2 participants