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

[feature] view::trim #102

Merged
merged 2 commits into from
Aug 8, 2017
Merged

[feature] view::trim #102

merged 2 commits into from
Aug 8, 2017

Conversation

h-2
Copy link
Member

@h-2 h-2 commented Aug 2, 2017

No description provided.

@h-2 h-2 assigned rrahn Aug 2, 2017
@h-2 h-2 requested review from rrahn and cpockrandt August 2, 2017 18:38
@h-2
Copy link
Member Author

h-2 commented Aug 2, 2017

@sarahet This contains some things you will want to use for your PR. I ended putting all things into one class which seems best (and also the friend operator| prevents namespace problems).

@h-2 h-2 force-pushed the feature/view_trim branch from 2ebb6d4 to ef10ea0 Compare August 3, 2017 12:10
@h-2 h-2 mentioned this pull request Aug 3, 2017
Merged
@h-2 h-2 force-pushed the feature/view_trim branch from ef10ea0 to c49464b Compare August 3, 2017 14:08
sarahet
sarahet reviewed Aug 4, 2017
View reviewed changes
include/seqan3/range/view/trim.hpp Outdated
*
* | | |
* |---------------------------------|-----------------------------------------------------------------------------|
* | concepts required of `irng_t` | `seqan3::input_range_concept<irng_t> && seqan3::quality_concept<value_type_t<irng_t>>>` |
Copy link
Member

Choose a reason for hiding this comment

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

seqan3::quality_concept<value_type_t<irng_t>>> has one > too much.

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch!

@h-2 h-2 force-pushed the feature/view_trim branch from c49464b to ef3b36e Compare August 4, 2017 13:13
@h-2
Copy link
Member Author

h-2 commented Aug 4, 2017 •
edited
Loading

DONT MERGE YET:

  • update the view properties table to be like Documentation/views #103
  • rename view_trim to trim_fn like ranges-v3 (foo_fn is the type of view::foo while view_foo is the type of the range returned by view::foo(input_range))

READY TO MERGE FROM MY POV

@h-2 h-2 force-pushed the feature/view_trim branch from ef3b36e to 4582c9c Compare August 7, 2017 14:41
@h-2 h-2 merged commit f991d50 into seqan:master Aug 8, 2017
@ddurai ddurai mentioned this pull request Jul 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarahet sarahet sarahet left review comments

@rrahn rrahn Awaiting requested review from rrahn

@cpockrandt cpockrandt Awaiting requested review from cpockrandt

Assignees

@rrahn rrahn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@h-2 @sarahet @rrahn