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

Adding hasSameBinaryContentAs #1732

Closed
wants to merge 1 commit into from

Conversation

@ngeor
Copy link
Contributor

ngeor commented Dec 16, 2019

Check List:

  • Fixes #1059
  • Unit tests : YES
  • Javadoc with a code example (on API only) : YES
Copy link
Owner

joel-costigliola left a comment

we need to deprecate hasSameContentAs for hasSameTextualContentAs to fix #1059 otherwise
just some minor comments, thanks @ngeor!

@scordio scordio changed the title Adding hasSameBinaryContentAs (Fixes #1059) Adding hasSameBinaryContentAs Dec 17, 2019
@scordio

This comment has been minimized.

Copy link
Collaborator

scordio commented Dec 17, 2019

Closing and reopening to trigger Travis again

@scordio scordio closed this Dec 17, 2019
@scordio scordio reopened this Dec 17, 2019
@ngeor ngeor force-pushed the ngeor:hasSameBinaryContentAs branch from b175c28 to 91eee5a Dec 17, 2019
@ngeor

This comment has been minimized.

Copy link
Contributor Author

ngeor commented Dec 17, 2019

we need to deprecate hasSameContentAs for hasSameTextualContentAs to fix #1059 otherwise
just some minor comments, thanks @ngeor!

Thanks, I processed the minor code changes (I should've read the Contributing guidelines first).
Just to double check, shall I introduce this new method hasSameTextualContentAs?

@joel-costigliola

This comment has been minimized.

Copy link
Owner

joel-costigliola commented Dec 17, 2019

Yes let's add hasSameTextualContentAs (a copy of hasSameContentAs)

@ngeor ngeor force-pushed the ngeor:hasSameBinaryContentAs branch from 91eee5a to 2fb6236 Dec 18, 2019
@ngeor

This comment has been minimized.

Copy link
Contributor Author

ngeor commented Dec 18, 2019

I added the new method and marked the old ones as deprecated. Some extra things:

  • I added a @since tag to the new methods
  • I added a TempFileUtil class because the createTempFile method was copy pasted around
  • Fixed a typo in the docs Turkisch -> Turkish
@ngeor ngeor force-pushed the ngeor:hasSameBinaryContentAs branch from 2fb6236 to de2bdf2 Dec 18, 2019
@joel-costigliola

This comment has been minimized.

Copy link
Owner

joel-costigliola commented Dec 21, 2019

Integrated thanks @ngeor! (I have added the same assertion for path)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.