Skip to content

Add documentation to transform_inclusive_scan and transform_exclusive_scan #103

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.

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

Merged
merged 2 commits into from
Jun 5, 2022

Conversation

jgopel
Copy link
Contributor

@jgopel jgopel commented Jun 5, 2022

No description provided.

Copy link
Member

@glenfe glenfe left a comment

Choose a reason for hiding this comment

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

You also need to remove transform_inclusive_scan from https://github.com/boostorg/algorithm/blob/develop/doc/algorithm.qbk#L238

i.e. The section named Not-yet-documented C++17 Algorithms

Also, please lose the "sparkles" and other emoji in the commit messages.

@jgopel jgopel changed the title 📝 [transform_inclusive_scan] Add documentation Add documentation to transform_inclusive_scan Jun 5, 2022
@jgopel jgopel force-pushed the transform-inclusive-scan-docs branch 2 times, most recently from 1c917b4 to 97284c7 Compare June 5, 2022 14:20
@jgopel
Copy link
Contributor Author

jgopel commented Jun 5, 2022

Hi @glenfe - Thanks for the feedback - I hadn't yet noticed the "Not-yet documented" section of that quickbook. Please let me know if there are any other changes I can make to help this land.

@jgopel jgopel changed the title Add documentation to transform_inclusive_scan Add documentation to transform_inclusive_scan and transform_exclusive_scan Jun 5, 2022
@glenfe
Copy link
Member

glenfe commented Jun 5, 2022

Thanks @jgopel . This looks better now. To be complete, it should probably also add sections for the now documented functions in the CXX17_inner_algorithms documentation section.

i.e. In this block: https://github.com/boostorg/algorithm/blob/develop/doc/algorithm.qbk#L100-L109 we need something like:

[section:transform_inclusive_scan transform_inclusive_scan]
[*[^[link boost.algorithm.transform_inclusive_scan    transform_inclusive_scan]    ] ]
Transforms elements from the input range with an operation and then combines
those transformed elements with an operation such that the n-1th element and
the nth element are combined. Inclusivity means that the nth element is included
in the nth combination.
[endsect:transform_inclusive_scan]

And a similar section for transform_exclusive_scan

@jgopel jgopel force-pushed the transform-inclusive-scan-docs branch from 2620876 to d2ffff4 Compare June 5, 2022 14:36
@jgopel
Copy link
Contributor Author

jgopel commented Jun 5, 2022

Done 🙂 Based on the existing entries in the quickbook, I kept the description a little shorter there and left the longer description for the linked page. If you'd prefer a different approach, just say the word!

Problem:
- There is no documentation for the existing functions. This will make
  it harder for users to consume these functions, especially as new
  variants are added.

Solution:
- Add documentation.
@jgopel jgopel force-pushed the transform-inclusive-scan-docs branch from d2ffff4 to 533856f Compare June 5, 2022 14:40
Copy link
Member

@glenfe glenfe left a comment

Choose a reason for hiding this comment

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

Looks good to me now - thanks.

Problem:
- There is no documentation for the existing functions. This will make
  it harder for users to consume these functions, espcially as new
  variants are added.

Solution:
- Add documentation.
@jgopel jgopel force-pushed the transform-inclusive-scan-docs branch from 533856f to 6b7a38f Compare June 5, 2022 23:25
@mclow mclow merged commit 00c6f1d into boostorg:develop Jun 5, 2022
@jgopel jgopel deleted the transform-inclusive-scan-docs branch June 6, 2022 01:32
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