Skip to content

Remove misleading comments#2639

Merged
danhoeflinger merged 2 commits intomainfrom
dev/dhoeflin/remove_misleading_comments
Mar 23, 2026
Merged

Remove misleading comments#2639
danhoeflinger merged 2 commits intomainfrom
dev/dhoeflin/remove_misleading_comments

Conversation

@danhoeflinger
Copy link
Copy Markdown
Contributor

@danhoeflinger danhoeflinger commented Mar 20, 2026

As far as I understand, these are not public headers, and shouldn't claim to be in their comments.

I reworded the reference to cppreference, as I don't believe we should be referring to that as documentation for our features directly. Happy to discuss this if someone objects.

I came across this in review of #2620, where someone was including one of these headers directly.

Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR cleans up header comment blocks under include/oneapi/dpl/internal/random_impl/ to avoid implying these internal headers are “public” headers, and rewords the Philox header’s reference to cppreference to be less prescriptive.

Changes:

  • Removed the “Abstract / Public header file …” comment blocks from multiple internal random implementation headers.
  • Replaced the Philox header’s multi-line “Abstract” block with a single descriptive link line.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
include/oneapi/dpl/internal/random_impl/weibull_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/uniform_real_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/uniform_int_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/subtract_with_carry_engine.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/random_common.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/philox_engine.h Rewords/replaces the abstract block with a descriptive cppreference link line.
include/oneapi/dpl/internal/random_impl/normal_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/lognormal_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/linear_congruential_engine.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/geometric_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/extreme_value_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/exponential_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/discard_block_engine.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/cauchy_distribution.h Removes misleading “public header” abstract comment.
include/oneapi/dpl/internal/random_impl/bernoulli_distribution.h Removes misleading “public header” abstract comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ElenaTyuleneva
ElenaTyuleneva previously approved these changes Mar 23, 2026
Copy link
Copy Markdown
Contributor

@ElenaTyuleneva ElenaTyuleneva left a comment

Choose a reason for hiding this comment

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

The changes look reasonable to me.

Co-authored-by: ElenaTyuleneva <elena.tyuleneva@intel.com>
@danhoeflinger danhoeflinger merged commit aaee06f into main Mar 23, 2026
23 checks passed
@danhoeflinger danhoeflinger deleted the dev/dhoeflin/remove_misleading_comments branch March 23, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants