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

Move the type traits submodule to utility #2265

Merged
merged 26 commits into from
Nov 23, 2020

Conversation

joergi-w
Copy link
Member

@joergi-w joergi-w commented Nov 17, 2020

For the reviewer: I strongly recommend to review commit-wise! I created separate commits for moving the files and creating the deprecation note, so the moved file contents do not appear (you see ~90% less).

This PR moves the files from core/type_traits to the position specified below (in the deleted paths there are still header files with a deprecation note).

all.hpp basic.hpp concept.hpp pre.hpputility/type_traits/
transformation_trait_or.hpputility/type_traits/detail/
function.hpputility/type_traits/function_traits.hpp and utility/detail/multi_invocable.hpp
lazy.hpputility/type_traits/lazy_conditional.hpp and core/detail/is_class_template_declarable.hpp
deferred_crtp_base.hpp template_inspection.hppcore/detail/
iterator.hppcore/detail/iterator_traits.hpp
range.hppcore/range/type_traits.hpp (remove range_compatible)
pack.hpp ⇒ deleted

part of seqan/product_backlog#160

@codecov
Copy link

codecov bot commented Nov 17, 2020

Codecov Report

Merging #2265 (f8cafc2) into master (b6e8e19) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2265   +/-   ##
=======================================
  Coverage   98.15%   98.15%           
=======================================
  Files         262      262           
  Lines       10815    10815           
=======================================
  Hits        10616    10616           
  Misses        199      199           
Impacted Files Coverage Δ
...ment/configuration/align_config_scoring_scheme.hpp 100.00% <ø> (ø)
...e/seqan3/alignment/matrix/alignment_coordinate.hpp 100.00% <ø> (ø)
...lude/seqan3/alignment/matrix/alignment_optimum.hpp 100.00% <ø> (ø)
include/seqan3/alignment/matrix/debug_matrix.hpp 98.07% <ø> (ø)
...ignment/matrix/detail/aligned_sequence_builder.hpp 100.00% <ø> (ø)
...etail/alignment_matrix_column_major_range_base.hpp 100.00% <ø> (ø)
.../matrix/detail/combined_score_and_trace_matrix.hpp 100.00% <ø> (ø)
...qan3/alignment/matrix/detail/coordinate_matrix.hpp 100.00% <ø> (ø)
...qan3/alignment/matrix/detail/trace_matrix_full.hpp 100.00% <ø> (ø)
...alignment/matrix/detail/two_dimensional_matrix.hpp 97.67% <ø> (ø)
... and 71 more

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 b6e8e19...f8cafc2. Read the comment docs.

@joergi-w joergi-w force-pushed the refactor/type_traits branch 3 times, most recently from 233ce7d to 2525633 Compare November 18, 2020 01:10
@smehringer
Copy link
Member

Please use the [MISC] tag for the commits :)

@joergi-w
Copy link
Member Author

Please use the [MISC] tag for the commits :)

Thanks! :)

@joergi-w joergi-w force-pushed the refactor/type_traits branch 4 times, most recently from 73a8d1b to 0176900 Compare November 19, 2020 10:13
@joergi-w joergi-w marked this pull request as ready for review November 19, 2020 10:43
@joergi-w joergi-w requested review from a team and eseiler and removed request for a team November 19, 2020 11:02
Copy link
Member

@eseiler eseiler left a comment

Choose a reason for hiding this comment

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

Can you re-add the #pragma once in the deprecated headers? You sometimes removed it.

include/seqan3/utility/type_traits/all.hpp Outdated Show resolved Hide resolved
@joergi-w
Copy link
Member Author

Can you re-add the #pragma once in the deprecated headers? You sometimes removed it.

Sure! I also created all.hpp for Utility module, in order to define the utility group for doxygen.

Copy link
Member

@eseiler eseiler 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

@eseiler eseiler requested review from a team and smehringer and removed request for a team November 19, 2020 13:07
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.

First batch :) I will review the rest tomorrow

{

/*!\brief An invocable wrapper that defers the instantiation of a crtp_base class.
* \ingroup type_traits
Copy link
Member

Choose a reason for hiding this comment

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

is this \ingroup command still correct? I think it has to be \ingroup core

Check all other ingroups in this file too

include/seqan3/core/detail/all.hpp Show resolved Hide resolved
namespace seqan3
{

/*!\addtogroup type_traits
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
/*!\addtogroup type_traits
/*!\addtogroup core


/*!\file
* \author Hannes Hauswedell <hannes.hauswedell AT fu-berlin.de>
* \brief Provides seqan3::type_list and auxiliary type traits.
Copy link
Member

Choose a reason for hiding this comment

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

This brief seems wrong 🤔


/*!\brief Determines whether a source_type is a specialisation of another template.
* \implements seqan3::unary_type_trait
* \ingroup type_traits
Copy link
Member

Choose a reason for hiding this comment

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

also check the ingroup commands in this file

namespace seqan3
{

/*!\addtogroup type_traits
Copy link
Member

Choose a reason for hiding this comment

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

check ingroup commands

// value_type
// ----------------------------------------------------------------------------

#ifdef SEQAN3_DEPRECATED_310
Copy link
Member

Choose a reason for hiding this comment

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

why did you deprecate all of this? I thought only range_compatible should be deprecated?

This would have been easier if it would have been done in a separate PR ;) for next time.

Copy link
Member Author

Choose a reason for hiding this comment

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

These #ifdef SEQAN3_DEPRECATED_310 were already in the type_traits/range.hpp before and I didn't decide to deprecate this. I simply moved the whole file without changing its content (except removing range_compatible in the following commit). If you look at the individual commits it should become clearer where I moved a file or where I changed content.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok! sorry I missed that :)

Comment on lines 22 to 23
SEQAN3_DEPRECATED_HEADER("This header is deprecated and will be removed in SeqAn-3.1. "
"Please #include <seqan3/utility/type_traits/all.hpp> instead.")
Copy link
Member

Choose a reason for hiding this comment

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

Please leave the one liner even though it exceeds the line length. If you break the line here, unfortunately the second line is not properly displayed to the user if triggered.

Same for all other files

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I didn't know... I'll change it!

@smehringer
Copy link
Member

Oh oh.. a lot of merge conflicts. hopefully just concering the inlcudes. I hope its fine if I review after you resolved them. :)

@joergi-w
Copy link
Member Author

Rebased on master.

@joergi-w
Copy link
Member Author

Rebased on master.

@eseiler eseiler merged commit 9dbfcb4 into seqan:master Nov 23, 2020
@joergi-w joergi-w deleted the refactor/type_traits branch November 23, 2020 14:11
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