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

#165 Remove all MatcherOf ctors except last one #223

Merged
merged 1 commit into from Jan 26, 2021

Conversation

baudoliver7
Copy link
Contributor

#165

  • Remove all constructors except last one
  • Fix all bugs generated

@codecov
Copy link

codecov bot commented Jan 17, 2021

Codecov Report

Merging #223 (9b0f9fa) into master (81bdf65) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #223   +/-   ##
=========================================
  Coverage     98.81%   98.82%           
- Complexity      137      139    +2     
=========================================
  Files            27       27           
  Lines           338      339    +1     
  Branches          4        5    +1     
=========================================
+ Hits            334      335    +1     
  Misses            4        4           
Impacted Files Coverage Δ Complexity Δ
...java/org/llorllale/cactoos/matchers/MatcherOf.java 100.00% <ø> (ø) 4.00 <0.00> (-10.00)
.../java/org/llorllale/cactoos/matchers/EndsWith.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
...ava/org/llorllale/cactoos/matchers/HasContent.java 100.00% <100.00%> (ø) 9.00 <4.00> (+2.00)
...java/org/llorllale/cactoos/matchers/HasString.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
.../java/org/llorllale/cactoos/matchers/IsNumber.java 100.00% <100.00%> (ø) 6.00 <5.00> (+5.00)
...in/java/org/llorllale/cactoos/matchers/IsText.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
...a/org/llorllale/cactoos/matchers/MatchesRegex.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
...ava/org/llorllale/cactoos/matchers/StartsWith.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
...va/org/llorllale/cactoos/matchers/TextMatcher.java 100.00% <100.00%> (ø) 9.00 <6.00> (+5.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 81bdf65...9b0f9fa. Read the comment docs.

@0crat
Copy link
Collaborator

0crat commented Jan 17, 2021

@victornoel/z everybody who has role REV is banned at #223; I won't be able to assign anyone automatically; consider assigning someone manually (as in §19), or invite more people (as in §51), or remove the job from the scope (as in §14)

@victornoel
Copy link
Collaborator

@0crat assign me

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.

@baudoliver7 a few comments and changes :)

@baudoliver7
Copy link
Contributor Author

@victornoel PR updated

@baudoliver7
Copy link
Contributor Author

baudoliver7 commented Jan 21, 2021

@victornoel Some suggestions have been made

@victornoel
Copy link
Collaborator

@baudoliver7 great thx

@victornoel
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 23, 2021

@rultor merge

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

@rultor
Copy link
Collaborator

rultor commented Jan 23, 2021

@rultor merge

@baudoliver7 @victornoel Oops, I failed. You can see the full log here (spent 7min)

[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-release-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-invoker-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-resources-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-jar-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-javadoc-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-site-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-source-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-plugin-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-surefire-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-release-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-war-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-resources-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.codehaus.mojo:versions-maven-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-site-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.doxia:doxia-module-markdown: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-source-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.eluder.coveralls:coveralls-maven-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.jacoco:jacoco-maven-plugin: checking for updates from oss.sonatype.org
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-surefire-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.apache.maven.plugins:maven-war-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.codehaus.mojo:versions-maven-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.jacoco:jacoco-maven-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] artifact org.eluder.coveralls:coveralls-maven-plugin: checking for updates from central
[\u001b[1;34mINFO\u001b[m] Generating "\u001b[1mJavadoc\u001b[m" report      \u001b[1m --- \u001b[0;32mmaven-javadoc-plugin:3.2.0:javadoc\u001b[m
[\u001b[1;33mWARNING\u001b[m] Error fetching link: https://github.com/yegor256/cactoos/apidocs. Ignored it.
[\u001b[1;33mWARNING\u001b[m] Error fetching link: http://hamcrest.org/JavaHamcrest/apidocs. Ignored it.
[\u001b[1;33mWARNING\u001b[m] Error fetching link: https://junit.org/junit5/apidocs. Ignored it.
[\u001b[1;33mWARNING\u001b[m] Error fetching link: https://junit.org/junit5/apidocs. Ignored it.
[\u001b[1;33mWARNING\u001b[m] Error fetching link: https://github.com/mockito/mockito/apidocs. Ignored it.
[\u001b[1;34mINFO\u001b[m] No previous run data found, generating javadoc.
[\u001b[1;34mINFO\u001b[m] 
3 warnings
[\u001b[1;33mWARNING\u001b[m] Javadoc Warnings
[\u001b[1;33mWARNING\u001b[m] javadoc: warning - Error fetching URL: http://docs.mockito.googlecode.com/hg/latest/
[\u001b[1;33mWARNING\u001b[m] javadoc: warning - Error fetching URL: http://www.h2database.com/javadoc/
[\u001b[1;33mWARNING\u001b[m] javadoc: warning - Error fetching URL: https://jolbox.com/
[\u001b[1;34mINFO\u001b[m] Generating "\u001b[1mJaCoCo Aggregate\u001b[m" report\u001b[1m --- \u001b[0;32mjacoco-maven-plugin:0.8.5:report-aggregate\u001b[m
[\u001b[1;34mINFO\u001b[m] Generating Sitemap.
[\u001b[1;34mINFO\u001b[m] Rendering \u001b[1m1 generated Doxia document\u001b[m: \u001b[1m1 xdoc\u001b[m
[\u001b[1;34mINFO\u001b[m] \u001b[1m------------------------------------------------------------------------\u001b[m
[\u001b[1;34mINFO\u001b[m] \u001b[1;32mBUILD SUCCESS\u001b[m
[\u001b[1;34mINFO\u001b[m] \u001b[1m------------------------------------------------------------------------\u001b[m
[\u001b[1;34mINFO\u001b[m] Total time:  01:52 min
[\u001b[1;34mINFO\u001b[m] Finished at: 2021-01-23T12:13:36Z
[\u001b[1;34mINFO\u001b[m] \u001b[1m------------------------------------------------------------------------\u001b[m
+ mv /home/r/repo .
++ whoami
+ chown -R root repo
+ '[' -n '' ']'
++ whoami
+ sudo chown -R rultor repo
+ cd repo
+ git push origin master
remote: error: GH006: Protected branch update failed for refs/heads/master.        
remote: error: 2 of 5 required status checks are failing.        
To git@github.com:llorllale/cactoos-matchers.git
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'git@github.com:llorllale/cactoos-matchers.git'
container c3f227331ba6fe10f2f2b281a918b96d9fead36e35205efebd390bd8196ad6b3 is dead
Sat Jan 23 13:14:04 CET 2021

@baudoliver7 baudoliver7 force-pushed the remove_all_constructors_except branch 2 times, most recently from a9aeeb3 to ebd3445 Compare January 23, 2021 13:20
@victornoel
Copy link
Collaborator

@baudoliver7 unfortunately, in cactoos-matchers, rultor is more capricious than in cactoos and doesn't want to merge if there are failed checks. I think if we manage to have tests covering those two lines it should work: https://github.com/llorllale/cactoos-matchers/pull/223/checks?check_run_id=1754033840

@baudoliver7
Copy link
Contributor Author

@victornoel Ok, I check

@baudoliver7 baudoliver7 force-pushed the remove_all_constructors_except branch 5 times, most recently from aff8dc6 to d36f89d Compare January 23, 2021 17:27
@baudoliver7
Copy link
Contributor Author

@victornoel Test coverage is now quite good :)

@baudoliver7
Copy link
Contributor Author

@victornoel Could you try to merge once again ? Thx :)

@victornoel
Copy link
Collaborator

@baudoliver7 yep thx :)

@victornoel
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Jan 26, 2021

@rultor merge

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

@rultor rultor merged commit 9b0f9fa into llorllale:master Jan 26, 2021
@rultor
Copy link
Collaborator

rultor commented Jan 26, 2021

@rultor merge

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

@0crat
Copy link
Collaborator

0crat commented Jan 26, 2021

Code review was too long (9 days), architects (@victornoel) were penalized, see §55

@0crat 0crat removed the 0crat/scope label Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants