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

Fix clang-format version check for 11.1.0 #9732

Closed
wants to merge 1 commit into from
Closed

Fix clang-format version check for 11.1.0 #9732

wants to merge 1 commit into from

Conversation

davidwendt
Copy link
Contributor

Fixes CI builds failing style checks which stop the build from compiling and executing tests.

@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf blocker libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Nov 19, 2021
@davidwendt davidwendt self-assigned this Nov 19, 2021
@davidwendt davidwendt requested a review from a team as a code owner November 19, 2021 13:38
@bdice
Copy link
Contributor

bdice commented Nov 19, 2021 •
edited
Loading

I created #9730 to solve the same problem by forward-merging @galipremsagar’s PR #9716. I think the forward merger is the desired way to fix this?

@davidwendt
Copy link
Contributor Author

I created #9730 to solve the same problem by forward-merging @galipremsagar’s PR #9716. I think the forward merger is the desired way to fix this?

Perhaps but then why is it failing in 22.02? Seems like something is wrong. This change unblocks me on my current PRs and does not appear to hold up 9730.

@bdice
Copy link
Contributor

bdice commented Nov 19, 2021

I created #9730 to solve the same problem by forward-merging @galipremsagar’s PR #9716. I think the forward merger is the desired way to fix this?

Perhaps but then why is it failing in 22.02? Seems like something is wrong. This change unblocks me on my current PRs and does not appear to hold up 9730.

@galipremsagar’s PR targeted 21.12 but had a merge conflict with 22.02, causing the error we see on 22.02. The forward-merger I created (with resolved conflicts from #9721) also includes the change in this PR.

bdice
bdice reviewed Nov 19, 2021
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

This PR does not correct the version numbers in the conda environment pinnings, so local environments created from that file would not match the pinned clang-format in CI. Forward-merging #9730 would fix the issue while keeping environments consistent.

@davidwendt
Copy link
Contributor Author

Fixed by #9730

@davidwendt davidwendt closed this Nov 19, 2021
@davidwendt davidwendt deleted the fix-clang-format-check branch November 22, 2021 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice left review comments

@ttnghia ttnghia ttnghia approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

Assignees

@davidwendt davidwendt

Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@davidwendt @bdice @ttnghia @jrhemstad