-
Notifications
You must be signed in to change notification settings - Fork 29
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
#80 GUI prototype #349
#80 GUI prototype #349
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slskiba thanks for this PR. You really have quickly jump started into the project and already implemented a working solution for the story 👍
As always I am picky and give you lots of review comments for improvement.
Also I am aware that you marked this PR as draft so it is WIP and maybe some of my points are already on your todo list...
I hope my comments help you to continue and improve.
Pull Request Test Coverage Report for Build 9496934290Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slskiba thanks for your rework. Looks nice now.
I added the java.version
to the top-level POM. Could you remove it from the cli/pom.xml
since it is redundant otherwise (could not do that easily in the browser).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slskiba thanks for perfectly addressing all my review comments.
Now ready for merge.
Closes #80
Shows a welcome message in the window.
Display the cli project's version in the window title as poc of module dependency.