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

Allow program to run as a traditional CLI #23

Merged
merged 12 commits into from Sep 30, 2018

Conversation

renatoathaydes
Copy link

In this PR, I 've added a simple ArgsParser class to allow user to pass in options to the program, so that they can compare two images from image files they pass in as arguments.
The user can also give a file where the result should be saved to. Not providing an output file makes the program open the UI, as it does currently.

I also made the jar created by Gradle be runnable (by adding the Main-Class entry to the manifest), so now you can run the program with java -jar image-comparison.jar.

renato added 6 commits September 29, 2018 19:23
Parse CLI options to know whether the user wants to see the demo (default), or compare two images and see the result (or save the result image to a file).
@coveralls
Copy link

coveralls commented Sep 29, 2018

Coverage Status

Coverage increased (+0.08%) to 96.407% when pulling 82542e9 on renatoathaydes:master into c58c124 on romankh3:master.

@renatoathaydes
Copy link
Author

That coverage decrease is pretty weird, given that almost the only thing I changed in ImageComparison, the class which accounts for all of the decrease, was the main function and new constructor! Not sure what kind of test would be useful for that!

Javadoc for public constructor (allows ImageComparison to be used as a library).
@renatoathaydes
Copy link
Author

Oh wow, I made the code a bit worse and now Coveralls is happy :D

@romankh3
Copy link
Owner

romankh3 commented Sep 30, 2018

Hello, @renatoathaydes. Good idea to do it.

I think this updates should be described in README.md according to the updates with running an application.

Coverage can be fixed if you create test with running ImageComparison.main method.

@renatoathaydes
Copy link
Author

The Coveralls report was green when I commented!! Looks like it changed later.
The main function is not testable unless you actually run the java process and capture the output to see if it's as expected, or use some UI testing tool that can detect the image displayed in the UI, both of which are massive overkill for those few lines of code in main!
I will break up main into smaller methods and try to test those... but if you don't mind me saying, this is waste of time, the project tests are quite adequate in my opinion...
I will add something on the README page.

@renatoathaydes
Copy link
Author

Looks like coveralls is happy now.
@romankh3 Can you publish this on Maven Central/JCenter?

If you need any help I can make a Pull Request with the necessary changes.
Here's a guide if you want to do that: https://blog.bintray.com/2014/02/11/bintray-as-pain-free-gateway-to-maven-central/

@romankh3
Copy link
Owner

You've done good work. I will accept your changes and merge it into master branch.

@romankh3 romankh3 merged commit d191555 into romankh3:master Sep 30, 2018
@romankh3
Copy link
Owner

@renatoathaydes, I don't have enough time to do that and don't have an experience in publishing to Maven Central.
Please, provide pull-request with required changes.

@renatoathaydes
Copy link
Author

Thanks.

Please, provide pull-request with required changes.

Sure, but first you'll have to get a Sonatype account and Bintray account. The link I gave above explain how you do that.

@romankh3
Copy link
Owner

@renatoathaydes Hello. I did something, but I'm not sure that it's what you want. I added a barge to README.md and you can see it.

Please, provide some feedback.

P.S. I wrote you an email message.

@renatoathaydes
Copy link
Author

You're almost done. There's no uploaded files here: https://bintray.com/romankh3/image-comparison/image-comparison/V2.0#files

You need to run gradlew bintrayUpload successfully for the files to be uploaded. Not sure why you're getting errors, try to ask someone on Stack Overflow maybe?

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