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

[MISC] Adds new concept template_specialisation_of. #2263

Merged
merged 1 commit into from
Dec 10, 2020
Merged

[MISC] Adds new concept template_specialisation_of. #2263

merged 1 commit into from
Dec 10, 2020

Conversation

Irallia
Copy link
Contributor

@Irallia Irallia commented Nov 16, 2020

Resolves partly seqan/product_backlog#259

@marehr
Copy link
Member

marehr commented Nov 16, 2020

Did you talk with @smehringer about this? Or did you pick this yourself?

@Irallia Irallia force-pushed the misc/core/add_concept_template_specialisation_of branch from 1bd55d3 to 448b139 Compare November 16, 2020 16:13
@Irallia
Copy link
Contributor Author

Irallia commented Nov 16, 2020

Did you talk with @smehringer about this? Or did you pick this yourself?

She gave me this issue.

@Irallia Irallia force-pushed the misc/core/add_concept_template_specialisation_of branch 2 times, most recently from bcdb87d to 56854fb Compare November 17, 2020 12:20
@codecov
Copy link

codecov bot commented Nov 17, 2020 •
edited
Loading

Codecov Report

Merging #2263 (32f602a) into master (c812a00) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2263   +/-   ##
=======================================
  Coverage   98.18%   98.18%           
=======================================
  Files         261      261           
  Lines       10820    10820           
=======================================
  Hits        10624    10624           
  Misses        196      196

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 c812a00...32f602a. Read the comment docs.

@Irallia Irallia marked this pull request as ready for review November 17, 2020 13:26
@Irallia Irallia requested a review from marehr November 17, 2020 13:26
@Irallia Irallia mentioned this pull request Nov 17, 2020
Merged
@marehr
Copy link
Member

marehr commented Nov 18, 2020

I guess this is blocked by #2265. We should first do the move of {core/type_traits/ => core/detail/}template_inspection.hpp

See https://gist.github.com/rrahn/f6b09ad67bebd14bd9c58f77c53e0450

marehr
marehr reviewed Nov 18, 2020
View reviewed changes
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.

Postponed until the header template_inspection.hpp was moved.

@smehringer smehringer mentioned this pull request Nov 23, 2020
Closed
8 tasks
@Irallia Irallia force-pushed the misc/core/add_concept_template_specialisation_of branch from 56854fb to a7ac1f5 Compare November 23, 2020 17:02
@Irallia Irallia requested a review from marehr November 23, 2020 17:04
@Irallia Irallia force-pushed the misc/core/add_concept_template_specialisation_of branch from a7ac1f5 to be14ef5 Compare November 24, 2020 08:47
@Irallia Irallia self-assigned this Dec 7, 2020
marehr
marehr approved these changes Dec 8, 2020
View reviewed changes
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

@marehr marehr requested review from rrahn and smehringer and removed request for rrahn December 8, 2020 10:51
smehringer
smehringer approved these changes Dec 9, 2020
View reviewed changes
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.

Just to tiny documentation things. After including them this can be merged.

include/seqan3/core/detail/template_inspection.hpp Outdated
*
* \ingroup type_traits
*
* \tparam mytype The source type.
Copy link
Member

@smehringer smehringer Dec 9, 2020 •
edited by eseiler
Loading

Choose a reason for hiding this comment

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

Suggested change
* \tparam mytype The source type.
* \tparam mytype The query type.

?
Source type somehow threw me off. Maybe query isn't the best alternative thought. The type to check ?
Source type sounds like I'm actually doing something with it or taking something from it to put somewhere else..

test/snippet/core/detail/template_inspection_usage_3.cpp Outdated
{
using my_type = std::vector<int>;

if constexpr (seqan3::detail::template_specialisation_of<my_type, std::vector>) // std::vector has no <> !
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if constexpr (seqan3::detail::template_specialisation_of<my_type, std::vector>) // std::vector has no <> !
if constexpr (seqan3::detail::template_specialisation_of<my_type, std::vector>) // Note: std::vector has no <> !

@Irallia Irallia force-pushed the misc/core/add_concept_template_specialisation_of branch from be14ef5 to 32f602a Compare December 9, 2020 14:07
@smehringer smehringer merged commit 89cf9df into seqan:master Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smehringer smehringer smehringer approved these changes

@marehr marehr marehr approved these changes

Assignees

@Irallia Irallia

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Irallia @marehr @smehringer