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.

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

[MISC] Replace seqan3::const_reference_t with std one #1729

Merged
merged 3 commits into from
Apr 14, 2020

Conversation

wvdtoorn
Copy link
Contributor

@wvdtoorn wvdtoorn commented Apr 9, 2020

Resolves part of #1549.

Split into two commits for readability.

@wvdtoorn wvdtoorn requested review from a team and marehr and removed request for a team April 9, 2020 05:56
@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

Merging #1729 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1729   +/-   ##
=======================================
  Coverage   97.68%   97.68%           
=======================================
  Files         237      238    +1     
  Lines        9057     9066    +9     
=======================================
+ Hits         8847     8856    +9     
  Misses        210      210           
Impacted Files Coverage Δ
include/seqan3/search/detail/search.hpp 100.00% <0.00%> (ø)
include/seqan3/search/configuration/max_error.hpp 100.00% <0.00%> (ø)
...e/seqan3/alignment/pairwise/detail/type_traits.hpp 100.00% <0.00%> (ø)
include/seqan3/search/detail/search_common.hpp 100.00% <0.00%> (ø)
include/seqan3/io/alignment_file/format_bam.hpp 95.47% <0.00%> (+0.01%) ⬆️
include/seqan3/argument_parser/validators.hpp 88.88% <0.00%> (+0.13%) ⬆️

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 89a77ba...ef161e4. Read the comment docs.

Copy link
Member

@marehr marehr left a comment

Choose a reason for hiding this comment

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

Thank you 👍

test/unit/core/type_traits/range_iterator_test.cpp Outdated Show resolved Hide resolved
test/unit/core/type_traits/range_iterator_test.cpp Outdated Show resolved Hide resolved
@wvdtoorn wvdtoorn requested a review from marehr April 9, 2020 11:41
@marehr marehr requested review from a team and smehringer and removed request for a team April 9, 2020 22:37
Copy link
Member

@smehringer smehringer left a comment

Choose a reason for hiding this comment

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

The change LGTM.

@marehr Why aren't we removing these tests since no seqan3 functionality is used anymore?
We don't ned to test the standard?

@marehr
Copy link
Member

marehr commented Apr 12, 2020

@smehringer good point, we should do this afterwards. I added this as acceptance criteria.

Copy link
Member

@smehringer smehringer left a comment

Choose a reason for hiding this comment

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

fine

@smehringer smehringer merged commit 332f4b3 into seqan:master Apr 14, 2020
wvdtoorn added a commit to wvdtoorn/seqan3 that referenced this pull request Apr 15, 2020
…eqan#1729)

* [MISC] Replace seqan3::const_reference_t with std ones

* [MISC] Remove double const_reference_ tests

* Bring back container test and style fix
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.

None yet

3 participants