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] Rename {core->utlity}/parallel/detail/spin_delay #2259

Merged
merged 2 commits into from
Nov 23, 2020
Merged

[MISC] Rename {core->utlity}/parallel/detail/spin_delay #2259

merged 2 commits into from
Nov 23, 2020

Conversation

MitraDarja
Copy link
Contributor

@codecov
Copy link

codecov bot commented Nov 12, 2020 •
edited
Loading

Codecov Report

Merging #2259 (efa8818) into master (9dabc3b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2259   +/-   ##
=======================================
  Coverage   98.15%   98.15%           
=======================================
  Files         262      262           
  Lines       10815    10815           
=======================================
  Hits        10616    10616           
  Misses        199      199
Impacted Files Coverage Δ
include/seqan3/core/parallel/detail/latch.hpp 96.77% <ø> (ø)
...an3/core/parallel/detail/reader_writer_manager.hpp 97.05% <ø> (ø)
...lude/seqan3/utility/parallel/detail/spin_delay.hpp 100.00% <100.00%> (ø)

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 9dabc3b...efa8818. Read the comment docs.

@MitraDarja MitraDarja requested review from a team and Irallia and removed request for a team November 12, 2020 22:57
Irallia
Irallia approved these changes Nov 14, 2020
View reviewed changes
Copy link
Contributor

@Irallia Irallia left a comment

Choose a reason for hiding this comment

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

LGFM, thank you!

@Irallia Irallia requested review from a team and rrahn and removed request for a team November 14, 2020 12:36
@MitraDarja
Copy link
Contributor Author

@rrahn polite ping :)

rrahn
rrahn requested changes Nov 17, 2020
View reviewed changes
include/seqan3/contrib/parallel/buffer_queue.hpp Outdated
@@ -21,7 +21,7 @@
#include <vector>

#include <seqan3/core/bit_manipulation.hpp>
#include <seqan3/core/parallel/detail/spin_delay.hpp>
#include <seqan3/utility/parallel/detail/spin_delay.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

include order

include/seqan3/core/parallel/detail/latch.hpp Outdated
@@ -15,7 +15,7 @@
#include <atomic>
#include <cassert>

#include <seqan3/core/parallel/detail/spin_delay.hpp>
#include <seqan3/utility/parallel/detail/spin_delay.hpp>
#include <seqan3/std/new>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, move this include to the stl includes.

include/seqan3/core/parallel/detail/reader_writer_manager.hpp
@@ -18,7 +18,7 @@
#include <seqan3/std/new>

#include <seqan3/core/parallel/detail/latch.hpp>
#include <seqan3/core/parallel/detail/spin_delay.hpp>
#include <seqan3/utility/parallel/detail/spin_delay.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

include order

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't really make sense here, the other files get moved to utillity as well in my next PR.

include/seqan3/core/parallel/detail/spin_delay.hpp
* \author Rene Rahn <rene.rahn AT fu-berlin.de>
* \deprecated This header will be removed in 3.1. Please use seqan3/utility/parallel/detail/spin_delay.hpp instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* \deprecated This header will be removed in 3.1. Please use seqan3/utility/parallel/detail/spin_delay.hpp instead.
* \deprecated This header will be removed in 3.1.0; Please \#include <seqan3/utility/parallel/detail/spin_delay.hpp> instead.

@MitraDarja
Copy link
Contributor Author

@rrahn Can you take a look at the CI? I don't understand why the changed include order now leads to an error on macOS.

@eseiler
Copy link
Member

eseiler commented Nov 19, 2020

@rrahn Can you take a look at the CI? I don't understand why the changed include order now leads to an error on macOS.

You need to rebase on master

rrahn
rrahn approved these changes Nov 23, 2020
View reviewed changes
Copy link
Contributor

@rrahn rrahn left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you

@rrahn rrahn merged commit 8a7e498 into seqan:master Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrahn rrahn rrahn approved these changes

@Irallia Irallia Irallia approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MitraDarja @eseiler @rrahn @Irallia