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.

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

Add support for new TDIGEST features and changes #2392

Merged
merged 6 commits into from
Sep 21, 2022
Merged

Conversation

dvora-h
Copy link
Collaborator

@dvora-h dvora-h commented Sep 20, 2022

Pull Request check-list

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

  • Does $ tox pass with this change (including linting)?
  • 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

Please provide a description of the change here.

@dvora-h dvora-h merged commit 027b452 into redis:master Sep 21, 2022
@dvora-h dvora-h deleted the tdigest branch September 21, 2022 08:53
@akx
Copy link
Contributor

akx commented Sep 21, 2022

Just curious (as a contributor to redis-py):

  • Why was "View Test Results" removed? That doesn't seem related to this PR's purpose.
  • Why isn't the PR checklist, or "Description of the change" filled out at all?

akx added a commit to akx/redis-py that referenced this pull request Sep 21, 2022
Since this is a public project, the token shouldn't be necessary
@akx akx mentioned this pull request Sep 21, 2022
6 tasks
@dvora-h
Copy link
Collaborator Author

dvora-h commented Sep 21, 2022

View Test Results removed because it failed the CI even when the tests passed and Codecov was accidentally removed.
Thanks for the attention!

@akx
Copy link
Contributor

akx commented Sep 21, 2022

Hi @dvora-h – can you show where View Test Results was failing CI? All I can find in the previous Checks results here are some async tests being flaky (the same thing that is currently failing CI in master and other PRs).

akx added a commit to akx/redis-py that referenced this pull request Sep 21, 2022
akx added a commit to akx/redis-py that referenced this pull request Sep 21, 2022
Since this is a public project, the token shouldn't be necessary
akx added a commit to akx/redis-py that referenced this pull request Sep 21, 2022
Since this is a public project, the token shouldn't be necessary
@dvora-h
Copy link
Collaborator Author

dvora-h commented Sep 21, 2022

@akx
Copy link
Contributor

akx commented Sep 21, 2022

@dvora-h Right, that seems to be an issue with the GHA tokens being used. dorny/test-reporter#149

akx added a commit to akx/redis-py that referenced this pull request Sep 21, 2022
Since this is a public project, the token shouldn't be necessary
dvora-h pushed a commit that referenced this pull request Sep 21, 2022
Since this is a public project, the token shouldn't be necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants