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

Include <optional> in headers that use std::optional #10044

Merged
merged 7 commits into from
Jan 20, 2022
Merged

Include <optional> in headers that use std::optional #10044

merged 7 commits into from
Jan 20, 2022

Conversation

robertmaynard
Copy link
Contributor

Detected when compiling with gcc-11 where wasn't being brought in by other standard headers

@robertmaynard
Include <optional> in headers that use std::optional
89a3694 
Detected when compiling with gcc-11 where <optional> wasn't
being brought in by other standard headers
@robertmaynard robertmaynard added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Jan 13, 2022
@robertmaynard robertmaynard requested a review from a team as a code owner January 13, 2022 20:48
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Jan 13, 2022
@robertmaynard
Copy link
Contributor Author

rerun tests

bdice
bdice approved these changes Jan 13, 2022
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.

LGTM.

@robertmaynard
Copy link
Contributor Author

rerun tests

@codecov
Copy link

codecov bot commented Jan 14, 2022 •
edited
Loading

Codecov Report

Merging #10044 (7b76535) into branch-22.02 (967a333) will decrease coverage by 0.08%.
The diff coverage is n/a.

❗ Current head 7b76535 differs from pull request most recent head 7c656a5. Consider uploading reports for the commit 7c656a5 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.02   #10044      +/-   ##
================================================
- Coverage         10.49%   10.40%   -0.09%     
================================================
  Files               119      119              
  Lines             20305    20556     +251     
================================================
+ Hits               2130     2139       +9     
- Misses            18175    18417     +242
Impacted Files Coverage Δ
python/custreamz/custreamz/kafka.py 29.16% <0.00%> (-0.63%) ⬇️
python/dask_cudf/dask_cudf/sorting.py 92.66% <0.00%> (-0.25%) ⬇️
python/dask_cudf/dask_cudf/core.py 70.85% <0.00%> (-0.17%) ⬇️
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/parquet.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/dtypes.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/scalar.py 0.00% <0.00%> (ø)
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fd7dd2...7c656a5. Read the comment docs.

@robertmaynard
Copy link
Contributor Author

rerun tests

@davidwendt davidwendt mentioned this pull request Jan 18, 2022
Merged
@davidwendt
Copy link
Contributor

Looks like another PR (#10067) has been opened that does this too.
From there it looks like a benchmark file has been missed in this PR.
https://github.com/rapidsai/cudf/pull/10067/files#diff-8ea6c0e0bf3596d2913e6ac09171c05f4fd18e88e5cfcb5897844f9aa5a8637bR34

@robertmaynard
Copy link
Contributor Author

Looks like another PR (#10067) has been opened that does this too. From there it looks like a benchmark file has been missed in this PR. https://github.com/rapidsai/cudf/pull/10067/files#diff-8ea6c0e0bf3596d2913e6ac09171c05f4fd18e88e5cfcb5897844f9aa5a8637bR34

I will revise. I was only building libcudf so I missed that

@robertmaynard
Copy link
Contributor Author

rerun tests

@robertmaynard
Copy link
Contributor Author

@gpucibot merge

@rongou
Copy link
Contributor

rongou commented Jan 19, 2022

rerun tests

@robertmaynard
Copy link
Contributor Author

CI failures are resolved by #10008

@vyasr
Copy link
Contributor

vyasr commented Jan 20, 2022

rerun tests

@rapids-bot rapids-bot bot merged commit 6bbe2e8 into rapidsai:branch-22.02 Jan 20, 2022
@robertmaynard robertmaynard deleted the bug/include_optional_where_used branch January 20, 2022 13:22
@bdice bdice mentioned this pull request Jan 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidwendt davidwendt davidwendt approved these changes

@ttnghia ttnghia ttnghia approved these changes

@bdice bdice bdice approved these changes

@vyasr vyasr vyasr approved these changes

@trxcllnt trxcllnt Awaiting requested review from trxcllnt trxcllnt is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
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.
6 participants
@robertmaynard @davidwendt @rongou @vyasr @bdice @ttnghia