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

Java solution (with TDD, no less) #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MilanVlaski
Copy link

@yegor256 Thank you for your great practical work. You've helped many to adopt object thinking!

I would appreciate some feedback on this pull request.

@yegor256
Copy link
Owner

@MilanVlaski looks very good! Way more object oriented than the original version :) I see that you've made changes here not only related to OOP redesign, but also simply related to the quality of documentation and XML configuration. Can you please make them in separate PRs? For example, the TODO.md file would be a nice addition to the README (don't make it separate). Also, the polishing of XML would be great to have in a separate PR.

I'm not going to merge your OOP changes, as you understand, because they will ruin the idea of this repo - other people must see the "wrong" version, not a "clean" one. But your other changes are very good and deserve to be merged.

@MilanVlaski
Copy link
Author

MilanVlaski commented May 21, 2024

I assume you want the TODO.md to be copied to the README.md.
Would that be two PR-s. For the todo and the xml?
Thanks for the fast feedback.
I must admit that I have no idea how Maven really works, I just copied the broken dependencies into the XML while getting rid of the parent project dependencies.

I'll make the pull requests most likely tomorrow. Good night.

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.

2 participants