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

(#1573) Generify org.cactoos.experimental package. #1627

Merged

Conversation

rocket-3
Copy link
Contributor

For #1573.

  • Generified parameter types in constructors of org.cactoos.experimental.Threads class
  • Removed todo comment

Actually one class and one constructor, which contract, in my opinion, could be relaxed.

@rocket-3 rocket-3 changed the title (#1573) Added wildcards to Threads ctor. (#1573) Generify constructor's parameter types for Experimental package. Sep 12, 2021
@rocket-3 rocket-3 changed the title (#1573) Generify constructor's parameter types for Experimental package. (#1573) Generify org.cactoos.Experimental package. Sep 12, 2021
@rocket-3 rocket-3 changed the title (#1573) Generify org.cactoos.Experimental package. (#1573) Generify org.cactoos.experimental package. Sep 12, 2021
@rocket-3
Copy link
Contributor Author

@0crat status

@rocket-3
Copy link
Contributor Author

@0crat, status

@0crat
Copy link
Collaborator

0crat commented Sep 17, 2021

Are you speaking to me or about me here; you must always start your message with my name if you want to address it to me, see §1

@victornoel
Copy link
Collaborator

@0crat status

@codecov-commenter
Copy link

Codecov Report

Merging #1627 (a14ab90) into master (445cf16) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1627      +/-   ##
============================================
- Coverage     90.12%   90.10%   -0.02%     
- Complexity     1602     1603       +1     
============================================
  Files           298      298              
  Lines          3745     3749       +4     
  Branches        122      122              
============================================
+ Hits           3375     3378       +3     
  Misses          337      337              
- Partials         33       34       +1     
Impacted Files Coverage Δ
...rc/main/java/org/cactoos/experimental/Threads.java 100.00% <ø> (ø)
src/main/java/org/cactoos/scalar/Solid.java 90.00% <0.00%> (-10.00%) ⬇️
src/main/java/org/cactoos/map/MapOf.java 100.00% <0.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 445cf16...a14ab90. Read the comment docs.

@rocket-3
Copy link
Contributor Author

@victornoel, how can I fix that failed code coverage check?

@victornoel
Copy link
Collaborator

@rocket-3 no need to, it's more of a glitch than an actual problem, minus 0.02% is the same as no changes. I never found how to prevent this unfortunately...

@victornoel
Copy link
Collaborator

@0crat status

@0crat
Copy link
Collaborator

0crat commented Sep 18, 2021

@0crat status (here)

@victornoel This is what I know about this job in C63314D6Z, as in §32:

@victornoel
Copy link
Collaborator

@0crat in

@rocket-3
Copy link
Contributor Author

@0crat status

@@ -188,7 +188,7 @@ public Threads(
* @param tasks The tasks to be executed concurrently.
*/
private Threads(
final Func<Iterable<Callable<T>>, Iterable<Future<T>>> fnc,
final Func<? super Iterable<Callable<T>>, ? extends Iterable<? extends Future<T>>> fnc,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rocket-3 I think we are missing some in there, for the first Iterable, the Callable as well as the Future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@victornoel OK, as I see, it's a private ctor., therefore, it should be remained intact, as I think.
So I changed the PR to just puzzle removal commit.
WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rocket-3 why not indeed

@rocket-3 rocket-3 force-pushed the issue-1573-generify-experimental-package branch from a14ab90 to baa9d82 Compare September 29, 2021 16:53
@victornoel
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Oct 2, 2021

@rultor merge

@victornoel OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 7f14139 into yegor256:master Oct 2, 2021
@rultor
Copy link
Collaborator

rultor commented Oct 2, 2021

@rultor merge

@victornoel Done! FYI, the full log is here (took me 9min)

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

5 participants