-
Notifications
You must be signed in to change notification settings - Fork 40
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
Removed Apache Http Client and Rest #2
Conversation
Merge pull request #1 from GoToTags/master
Why did you remove the JsonModel related functions? |
@edvin Those json model functions are in the Rest.kt file. Not actually sure why they are there in the first place. I started by deleting that file and fixing compilation errors. Note that the next MR I was going to submit was removing the Json dependency. That one is more complicated as it deeply integrated with the configuration code. I also think we should remove the specific dependency on FontAwesome, not the core code that supports it though. This causes us an issue as we pay for FontAwesome and have a separate version than what is included in tornadofx. My belief is that tornadofx should enable the easy use of font icons, but not have specific support for any of them in core. We should also decide where this non-core code should go. This repository but a separate project? If so, how does that effect deployment? Separate repository? |
I agree removing the FontAwesome dependency is a good thing. The json dependencies in Configuration and other places could be added to the json module as extension functions, couldn't they? A separate project for each of the modules we extract sounds like a good way to go. |
@edvin Want to be really clear on this. Does this structure/naming work for you? Each module would be published separately. One repository
|
The current /src root now is there because of the Maven convention, not sure it is a good idea to use the same for modules. What about just putting the modules in /?
Alternatively, a |
@edvin I could have been more clear. Here is the full example. Personally I like all modules to be siblings of each other, and having all source code under a main
|
I understood what you meant :) OK, I'm fine with it! |
OK! This should enable us to make some progress on this. Wasn't expecting you to merge this in just yet. Ill put in other PRs for the folder restructure and other modules. |
Sorry about that :) I'll leave the merge to you from now on, will just comment on the merge requests. |
@ctadlock For future note, you can mark your PRs as drafts if they're not actually ready to be merged. |
This is being removed from the core of tornadofx; should be added later as a separate and optional item. Reasoning is that not all apps will want to use this specific Http client.
This is for: #3