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

upgrade spdlog to 1.8.5 #658

Merged
merged 16 commits into from
May 24, 2021
Merged

upgrade spdlog to 1.8.5 #658

merged 16 commits into from
May 24, 2021

Conversation

rongou
Copy link
Contributor

@rongou rongou commented Dec 11, 2020

The new version includes fmt version 7.1.3 which fixed the problem with cuda 10.x.

@harrism @kkraus14

@rongou rongou requested review from a team as code owners December 11, 2020 20:14
@rongou rongou added 3 - Ready for review Ready for review by team CMake conda improvement Improvement / enhancement to an existing function non-breaking Non-breaking change tech debt debt Internal clean up and improvements to reduce maintenance and technical debt in general labels Dec 11, 2020
@rongou rongou mentioned this pull request Dec 11, 2020
Merged
@kkraus14 kkraus14 added the 5 - Merge After Dependencies Depends on another PR: do not merge out of order label Dec 11, 2020
kkraus14
kkraus14 suggested changes Dec 11, 2020
View reviewed changes
Copy link
Contributor

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, do we want to leave the pinning open ended like this? Would we be better off just pinning to a specific version like 1.8.2 for now?

@rongou
Copy link
Contributor Author

rongou commented Dec 11, 2020

@kkraus14 Looks like we have to first upgrade the integration repo, so it's a two step process. I'd prefer we keep rolling forward.

mike-wendt
mike-wendt suggested changes Dec 14, 2020
View reviewed changes
Copy link
Contributor

@mike-wendt mike-wendt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update with pinnings that are decided on in rapidsai/integration#198

@mike-wendt
Copy link
Contributor

rerun tests

mike-wendt
mike-wendt approved these changes Dec 16, 2020
View reviewed changes
Copy link
Contributor

@mike-wendt mike-wendt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but needs to wait until rapidsai/integration#198 is merged with the conda pkg fix detailed in that PR

@mike-wendt
Copy link
Contributor

Approved, but needs to wait until rapidsai/integration#198 is merged with the conda pkg fix detailed in that PR

Until then the testing in this PR will continue to fail due to the upstream conda-forge changes

@rongou
Copy link
Contributor Author

rongou commented Jan 6, 2021

rerun tests

@harrism
Copy link
Member

harrism commented Apr 22, 2021

@rongou do you want to submit a PR to upgrade it in the integration repo?

@rongou
Copy link
Contributor Author

rongou commented Apr 22, 2021

It was done before: rapidsai/integration#198

@rongou
Copy link
Contributor Author

rongou commented Apr 22, 2021

rerun tests

@jakirkham jakirkham mentioned this pull request Apr 28, 2021
Merged
5 tasks
@ajschmidt8 ajschmidt8 changed the title upgrade spdlog to 1.8.5 upgrade spdlog to 1.8.5 May 21, 2021
@ajschmidt8 ajschmidt8 changed the title upgrade spdlog to 1.8.5 upgrade spdlog to 1.8.5 May 21, 2021
@rongou
Copy link
Contributor Author

rongou commented May 21, 2021

Can this be merged as is, or do I need to take out the testing changes in ci/gpu/build.sh?

@ajschmidt8
Copy link
Member

Can this be merged as is, or do I need to take out the testing changes in ci/gpu/build.sh?

@rongou, please remove the testing change. should be good after that

@kkraus14 kkraus14 removed the 5 - Merge After Dependencies Depends on another PR: do not merge out of order label May 21, 2021
@kkraus14
Copy link
Contributor

rerun tests

@kkraus14 kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for review Ready for review by team labels May 21, 2021
@rongou
Copy link
Contributor Author

rongou commented May 23, 2021

rerun tests

@kkraus14 kkraus14 dismissed raydouglass’s stale review May 24, 2021 03:34

integration changes addressed

@kkraus14
Copy link
Contributor

@rapidsai/ops this needs a review from you, but should be good to go

@ajschmidt8
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit a4c21d7 into rapidsai:branch-21.06 May 24, 2021
@rongou rongou deleted the upgrade-spdlog branch October 8, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harrism harrism harrism approved these changes

@mike-wendt mike-wendt mike-wendt approved these changes

@kkraus14 kkraus14 kkraus14 approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@codereport codereport Awaiting requested review from codereport

@raydouglass raydouglass Awaiting requested review from raydouglass

Assignees
No one assigned
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge CMake conda gpuCI improvement Improvement / enhancement to an existing function non-breaking Non-breaking change tech debt debt Internal clean up and improvements to reduce maintenance and technical debt in general
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rongou @mike-wendt @kkraus14 @raydouglass @harrism @ajschmidt8