Doc/create all hpp #2841

SGSSGene · 2021-10-05T15:00:54Z

Added a script that creates all.hpp file. It also checks already checked scripts.
The structure of an all.hpp files is assumed to be as follow:

<License text>
<Doxygen description of the modules>
#pragma once

#include <xyz/subpath1>
...

This PR has 3 big changes (=commits)

1. add a script that could generate proper all.hpp files
1. changing a few snippets that otherwise would break when using the new correct all.hpp files
1. adjusting all all.hpp by using the script from the first commit.

vercel · 2021-10-05T15:00:59Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/seqan/seqan3/BeogHETRhds7NAdvegnCoLBzRWiy
✅ Preview: https://seqan3-git-fork-sgssgene-doc-createallhpp-seqan.vercel.app

codecov · 2021-10-05T16:13:20Z

Codecov Report

Merging #2841 (5f2392d) into release-3.1.0 (305697e) will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           release-3.1.0    #2841   +/-   ##
==============================================
  Coverage          98.21%   98.21%           
==============================================
  Files                272      272           
  Lines              10827    10827           
==============================================
  Hits               10634    10634           
  Misses               193      193

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 305697e...5f2392d. Read the comment docs.

include/seqan3/alignment/all.hpp

SGSSGene · 2021-10-06T07:22:23Z

Note: The file alignment/matrix/all.hpp changes in a weird/interesting way.
Not sure if that is really what we want:
https://github.com/seqan/seqan3/pull/2841/files#diff-38d35830137a776cb0e96c207990c9ebee6773806ecde07e0c19a6d77a786046

marehr · 2021-10-06T22:54:31Z

Note: The file alignment/matrix/all.hpp changes in a weird/interesting way. Not sure if that is really what we want: https://github.com/seqan/seqan3/pull/2841/files#diff-38d35830137a776cb0e96c207990c9ebee6773806ecde07e0c19a6d77a786046

If it isn't too complicated, we should not generate and include detail/all.hpp, but I wouldn't be too sad if that can't be excluded.

SGSSGene · 2021-10-07T10:26:40Z

Note: The file alignment/matrix/all.hpp changes in a weird/interesting way. Not sure if that is really what we want: https://github.com/seqan/seqan3/pull/2841/files#diff-38d35830137a776cb0e96c207990c9ebee6773806ecde07e0c19a6d77a786046

If it isn't too complicated, we should not generate and include detail/all.hpp, but I wouldn't be too sad if that can't be excluded.

So I have changed it as follows:

Not generating all.hpp for folders including /detail or `/exposition_only' in their name
Only include detail/all.hpp if it already exists. (See include/seqan3/core/range/all.hpp as an example ).
Or would you rather never have any "#include <detail/all.hpp>" ?

include/seqan3/alignment/pairwise/all.hpp

SGSSGene · 2021-11-01T09:08:21Z

@eseiler Yes, I agree. My first version also worked like this.
The motivation to include it any ways is that some headers like include/seqan3/utility/simd/all.hpp include detail files and code breaks when they are removed:
https://github.com/seqan/seqan3/pull/2841/files#diff-1ef6e14184c93247afa48a4c90935202196a074cdaba8faa73ee2b0d25a2d162L13-L22

smehringer · 2021-11-02T05:53:22Z

include/seqan3/alignment/configuration/all.hpp

@@ -26,19 +42,3 @@
 #include <seqan3/alignment/configuration/align_config_scoring_scheme.hpp>
 #include <seqan3/alignment/configuration/align_config_vectorised.hpp>
 #include <seqan3/alignment/configuration/detail.hpp>


oh spotted a detail here? I guess you just added inlcudes but do not touch existing ones. fine for now in the scope of this PR! This can be a follow up.

Mhm, interessting. The issue is, that I didn't plan for detail.hpp only for detail folders....
Nice spot!
Will fix it now otherwise I forget.

include/seqan3/alphabet/all.hpp

smehringer · 2021-11-02T05:54:42Z

you need to review approvals to get merged, so I'm assigning team first.
Quick sweep looked good.

include/seqan3/contrib/charconv/all.hpp

SGSSGene · 2021-11-02T09:05:25Z

include/seqan3/alignment/configuration/all.hpp

@@ -26,19 +42,3 @@
 #include <seqan3/alignment/configuration/align_config_scoring_scheme.hpp>
 #include <seqan3/alignment/configuration/align_config_vectorised.hpp>
 #include <seqan3/alignment/configuration/detail.hpp>


Mhm, interessting. The issue is, that I didn't plan for detail.hpp only for detail folders....
Nice spot!
Will fix it now otherwise I forget.

Irallia

So many missing includes 😮
I have some questions, the most important, what happend to include/seqan3/utility/parallel/all.hpp?

include/seqan3/alignment/configuration/all.hpp

include/seqan3/utility/parallel/all.hpp

test/snippet/utility/simd/detail/default_simd_max_length.cpp

Irallia

LGFM, nice work!

eseiler · 2021-11-04T12:58:06Z

You need to add the debug_stream include to some of the SIMD snippets

SGSSGene · 2021-11-04T13:28:55Z

You need to add the debug_stream include to some of the SIMD snippets

Yes, I noticed :-) reverted the change!

smehringer · 2021-11-04T15:08:52Z

test/scripts/create_all_hpp.sh

+{
+cat << EOF
+// -----------------------------------------------------------------------------------------------------
+// Copyright (c) 2006-2021, Knut Reinert & Freie Universität Berlin


not important at all. But since this is a script anyway. Can we make it "The current year" instead of 2021?

Since everyhting else is done. this can be a follow up (if done at all)

Good idea, of course we can make it the current year, this way this script doesn't have to be updated :-)

I just want to point out, that we have also update_copyright.sh which should be used to actually update the copyright year, since it will change it in all files and not just all.hpp files.

ah you are right. then I think we do not need it

smehringer · 2021-11-04T15:13:55Z

needs a rebase though!

smehringer

LGTM

SGSSGene changed the base branch from master to release-3.1.0 October 5, 2021 15:01

smehringer reviewed Oct 6, 2021

View reviewed changes

include/seqan3/alignment/all.hpp Outdated Show resolved Hide resolved

SGSSGene force-pushed the doc/create_all_hpp branch from 62c3f42 to 9bdc2b1 Compare October 6, 2021 07:17

SGSSGene force-pushed the doc/create_all_hpp branch 2 times, most recently from 9f58d8a to c6d454c Compare October 6, 2021 08:18

SGSSGene force-pushed the doc/create_all_hpp branch from c6d454c to f00db8c Compare October 7, 2021 10:21

SGSSGene force-pushed the doc/create_all_hpp branch 3 times, most recently from 3efdda3 to 7b0d32f Compare October 7, 2021 14:04

SGSSGene force-pushed the doc/create_all_hpp branch from 7b0d32f to 978cc38 Compare October 18, 2021 10:20

SGSSGene force-pushed the doc/create_all_hpp branch from 978cc38 to 2c80957 Compare November 1, 2021 08:09

eseiler reviewed Nov 1, 2021

View reviewed changes

include/seqan3/alignment/pairwise/all.hpp Outdated Show resolved Hide resolved

SGSSGene force-pushed the doc/create_all_hpp branch from 8039604 to 9e34925 Compare November 1, 2021 12:21

SGSSGene requested a review from smehringer November 1, 2021 13:02

smehringer reviewed Nov 2, 2021

View reviewed changes

include/seqan3/alphabet/all.hpp Show resolved Hide resolved

smehringer requested review from a team and Irallia and removed request for a team November 2, 2021 05:54

SGSSGene commented Nov 2, 2021

View reviewed changes

Irallia requested changes Nov 2, 2021

View reviewed changes

include/seqan3/alignment/configuration/all.hpp Show resolved Hide resolved

include/seqan3/utility/parallel/all.hpp Outdated Show resolved Hide resolved

test/snippet/utility/simd/detail/default_simd_max_length.cpp Show resolved Hide resolved

SGSSGene added this to the SeqAn 3.1.0 milestone Nov 4, 2021

SGSSGene force-pushed the doc/create_all_hpp branch 2 times, most recently from 206d189 to db415f1 Compare November 4, 2021 10:31

SGSSGene requested a review from Irallia November 4, 2021 10:36

Irallia approved these changes Nov 4, 2021

View reviewed changes

Irallia requested review from a team and smehringer and removed request for a team November 4, 2021 11:08

SGSSGene force-pushed the doc/create_all_hpp branch from db415f1 to 9287c62 Compare November 4, 2021 13:28

smehringer approved these changes Nov 4, 2021

View reviewed changes

SGSSGene force-pushed the doc/create_all_hpp branch from 9287c62 to 4e1b34d Compare November 4, 2021 19:09

SGSSGene added 3 commits November 4, 2021 20:22

[INFRA] scripts that creates all.hpp files

d8b4cfa

[FIX] add missing detail/debug_stream_simd.hpp includes

7c0a750

[FIX] adjust all all.hpp files

5f2392d

SGSSGene force-pushed the doc/create_all_hpp branch from 4e1b34d to 5f2392d Compare November 4, 2021 19:22

SGSSGene requested a review from smehringer November 4, 2021 19:25

smehringer merged commit a22a18b into seqan:release-3.1.0 Nov 5, 2021

smehringer reviewed Nov 5, 2021

View reviewed changes

smehringer mentioned this pull request Nov 6, 2021

Script that automatically creates all.hpp files seqan/product_backlog#400

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doc/create all hpp #2841

Doc/create all hpp #2841

SGSSGene commented Oct 5, 2021 •

edited

Loading

vercel bot commented Oct 5, 2021 •

edited

Loading

codecov bot commented Oct 5, 2021 •

edited

Loading

SGSSGene commented Oct 6, 2021

marehr commented Oct 6, 2021

SGSSGene commented Oct 7, 2021

SGSSGene commented Nov 1, 2021

smehringer Nov 2, 2021

SGSSGene Nov 2, 2021

smehringer commented Nov 2, 2021

SGSSGene Nov 2, 2021

Irallia left a comment

Irallia left a comment

eseiler commented Nov 4, 2021

SGSSGene commented Nov 4, 2021

smehringer Nov 4, 2021

smehringer Nov 4, 2021

SGSSGene Nov 4, 2021

smehringer Nov 5, 2021

smehringer commented Nov 4, 2021

smehringer left a comment

Doc/create all hpp #2841

Doc/create all hpp #2841

Conversation

SGSSGene commented Oct 5, 2021 • edited Loading

vercel bot commented Oct 5, 2021 • edited Loading

codecov bot commented Oct 5, 2021 • edited Loading

Codecov Report

SGSSGene commented Oct 6, 2021

marehr commented Oct 6, 2021

SGSSGene commented Oct 7, 2021

SGSSGene commented Nov 1, 2021

smehringer Nov 2, 2021

Choose a reason for hiding this comment

SGSSGene Nov 2, 2021

Choose a reason for hiding this comment

smehringer commented Nov 2, 2021

SGSSGene Nov 2, 2021

Choose a reason for hiding this comment

Irallia left a comment

Choose a reason for hiding this comment

Irallia left a comment

Choose a reason for hiding this comment

eseiler commented Nov 4, 2021

SGSSGene commented Nov 4, 2021

smehringer Nov 4, 2021

Choose a reason for hiding this comment

smehringer Nov 4, 2021

Choose a reason for hiding this comment

SGSSGene Nov 4, 2021

Choose a reason for hiding this comment

smehringer Nov 5, 2021

Choose a reason for hiding this comment

smehringer commented Nov 4, 2021

smehringer left a comment

Choose a reason for hiding this comment

SGSSGene commented Oct 5, 2021 •

edited

Loading

vercel bot commented Oct 5, 2021 •

edited

Loading

codecov bot commented Oct 5, 2021 •

edited

Loading