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

#243: Introduce IsBytes #244

Merged
merged 3 commits into from May 15, 2021
Merged

#243: Introduce IsBytes #244

merged 3 commits into from May 15, 2021

Conversation

andreoss
Copy link
Contributor

@andreoss andreoss commented May 9, 2021

Per #243

  • Add IsBytes

@codecov
Copy link

codecov bot commented May 9, 2021

Codecov Report

Merging #244 (7abf3fc) into master (37cac65) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #244      +/-   ##
============================================
+ Coverage     98.95%   99.00%   +0.04%     
- Complexity      150      157       +7     
============================================
  Files            28       29       +1     
  Lines           383      401      +18     
  Branches          7        8       +1     
============================================
+ Hits            379      397      +18     
  Misses            4        4              
Impacted Files Coverage Δ Complexity Δ
...n/java/org/llorllale/cactoos/matchers/IsBytes.java 100.00% <100.00%> (ø) 7.00 <7.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 37cac65...7abf3fc. Read the comment docs.

@andreoss andreoss changed the title (#243) Introduce IsBytes #243: Introduce IsBytes May 9, 2021
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.

@andreoss see the comment and also, I don't think it's a good idea to have a bunch of constructors with lots of behaviour (in particular the int one). Instead let's just take a Bytes as well as a byte... and let the user exploit BytesOf if they need it.

src/main/java/org/llorllale/cactoos/matchers/IsBytes.java Outdated Show resolved Hide resolved
void mismatches() {
new Assertion<>(
"Must mismatch with bytes",
new IsBytes((byte) 65, (byte) 66, (byte) 67),
Copy link
Collaborator

Choose a reason for hiding this comment

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

@andreoss why do we need to cast here? if this is because of the char... constructor, then let's remove it, no?

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 it's narrowing conversion, casting should be explicit

@victornoel
Copy link
Collaborator

@rultor merge

@rultor
Copy link
Collaborator

rultor commented May 15, 2021

@rultor merge

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

@rultor rultor merged commit 7abf3fc into llorllale:master May 15, 2021
@rultor
Copy link
Collaborator

rultor commented May 15, 2021

@rultor merge

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

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

3 participants