Skip to content

[REVIEW] updated clang version to 11.0.0 - #170

Closed
teju85 wants to merge 2 commits into
rapidsai:branch-21.08from
teju85:fea-ext-update-clang-version
Closed

[REVIEW] updated clang version to 11.0.0#170
teju85 wants to merge 2 commits into
rapidsai:branch-21.08from
teju85:fea-ext-update-clang-version

Conversation

@teju85

@teju85 teju85 commented Nov 6, 2020

Copy link
Copy Markdown
Contributor

Why update?

If we want to enable running clang-tidy on .cu files, v8.0.1 does not even support CTK version 10, as described here: NVIDIA/raft#85. Thus, we need to update clang version.

Any risks due to this update?

None so far. I have tested this by running clang-tidy and clang-format on both cuML and raft repo's:

  1. [REVIEW] updated clang version to 11.0.0 NVIDIA/cuml#3121
  2. [DO NOT MERGE] updated clang to v11.0.0 NVIDIA/raft#89

Specifically, the set of changes caused by clang-format is almost negligible.

@teju85
teju85 requested a review from a team as a code owner November 6, 2020 05:26
@teju85

teju85 commented Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

@codereport, can you check if the above claims are true for cudf too? (meaning, clang update doesn't cause much of clang-format violations)

@teju85

teju85 commented Nov 9, 2020

Copy link
Copy Markdown
Contributor Author

@codereport also tried upgrading to clang-11 in his PR here: NVIDIA/cudf#6695 and saw not much changes in cuDF too. So, it seems like it is safer to upgrade clang version on our meta recipe.

@teju85

teju85 commented Nov 9, 2020

Copy link
Copy Markdown
Contributor Author

Tagging relevant folks to let us know if they are ok with this version upgrade:

  1. for cugraph, @afender or @seunghwak
  2. for rmm, cuspatial @harrism or @codereport

@mike-wendt mike-wendt added 4 - Needs Reviewer Waiting for reviewer to review or respond DO NOT MERGE Hold off on merging; see PR for details and removed 4 - Needs Reviewer Waiting for reviewer to review or respond labels Dec 3, 2020
@mike-wendt

Copy link
Copy Markdown
Contributor

From @kkraus14 punt to 0.18

@mike-wendt
mike-wendt changed the base branch from branch-0.17 to branch-0.18 December 9, 2020 22:47
@raydouglass
raydouglass changed the base branch from branch-0.18 to branch-0.19 March 4, 2021 13:58
@raydouglass

Copy link
Copy Markdown
Contributor

@teju85 Is this still needed?

@teju85

teju85 commented Mar 10, 2021

Copy link
Copy Markdown
Contributor Author

@raydouglass Yes. This is still needed. However, we require a "go ahead" signal from the above code owners to proceed.

@ajschmidt8
ajschmidt8 changed the base branch from branch-0.19 to branch-0.20 April 23, 2021 19:36
@ajschmidt8
ajschmidt8 changed the base branch from branch-21.06 to branch-21.08 June 9, 2021 16:59
@ajschmidt8

Copy link
Copy Markdown
Member

superseded by #390. closing

@ajschmidt8 ajschmidt8 closed this Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO NOT MERGE Hold off on merging; see PR for details

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants