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

(#1566) Generify org.cactoos.list package #1635

Merged

Conversation

DmitryBarskov
Copy link
Contributor

@DmitryBarskov DmitryBarskov commented May 6, 2022

For #1566:

Generify some classes in org.cactoos.list package:

  • org.cactoos.list.Immutable
  • org.cactoos.list.ImmutableListIterator
  • org.cactoos.list.Joined
  • org.cactoos.list.JoinedListIterator

@DmitryBarskov DmitryBarskov marked this pull request as ready for review May 6, 2022 10:15
@DmitryBarskov
Copy link
Contributor Author

Hey @victornoel! Could you have a look please?

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #1635 (fb5fccb) into master (2d3d898) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1635      +/-   ##
============================================
+ Coverage     90.10%   90.13%   +0.02%     
- Complexity     1603     1604       +1     
============================================
  Files           298      298              
  Lines          3749     3749              
  Branches        122      122              
============================================
+ Hits           3378     3379       +1     
  Misses          337      337              
+ Partials         34       33       -1     
Impacted Files Coverage Δ
src/main/java/org/cactoos/list/Joined.java 100.00% <ø> (ø)
src/main/java/org/cactoos/list/Immutable.java 100.00% <100.00%> (ø)
...n/java/org/cactoos/list/ImmutableListIterator.java 91.66% <100.00%> (ø)
...main/java/org/cactoos/list/JoinedListIterator.java 75.60% <100.00%> (ø)
src/main/java/org/cactoos/scalar/Solid.java 100.00% <0.00%> (+10.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 2d3d898...fb5fccb. Read the comment docs.

@yegor256
Copy link
Owner

@victornoel what's up with this?

Copy link
Collaborator

@victornoel victornoel left a comment

Choose a reason for hiding this comment

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

@DmitryBarskov late review :)

@@ -47,7 +47,7 @@
/**
* {@link List} of {@link ListIterator}.
*/
private final List<ListIterator<T>> listiters;
private final List<ListIterator<? extends T>> listiters;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@DmitryBarskov I think that should be List<? extends ListIterator<..., here and everywhere else :)

@victornoel
Copy link
Collaborator

@DmitryBarskov thank you. Did you verify that there was no more class in the list package that would need changes like those?

@DmitryBarskov
Copy link
Contributor Author

@DmitryBarskov thank you. Did you verify that there was no more class in the list package that would need changes like those?

@victornoel yes, checked it twice

@victornoel
Copy link
Collaborator

@DmitryBarskov excellent, thank you very much!

@victornoel
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jul 20, 2022

@rultor merge

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

@rultor rultor merged commit fdca9c3 into yegor256:master Jul 20, 2022
@rultor
Copy link
Collaborator

rultor commented Jul 20, 2022

@rultor merge

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

@DmitryBarskov DmitryBarskov deleted the issue-1566-generify-list-package branch July 20, 2022 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.

None yet

5 participants