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

FxmlViewLoader now uses ViewType to load resources #555

Conversation

mateusz-lisik
Copy link

Change is made in order to load FXML files across modules. This implementation was made in a way that doesn't affect java 8 runtime.

@manuel-mauky
Copy link
Collaborator

Hi @matteprl
thank you very much for this Pull Request.
One question: Is there any way to test this behaviour with JUnit? My concern is that if we don't have a test case then in the future a refactoring might break this behaviour without us noticing.

I assume that creating a separate maven module with a View/ViewModel and using it in the core module in a test case might be enough? Would this break on Java 9 without your change?
We would need to add Java 9 (or higher) to the travis-ci builds then but this would be a good idea anyway.

@mateusz-lisik
Copy link
Author

@lestard That's very valid point. I was trying to figure out how to write such test. Perhaps creating module that stores some views solely for testing purposes may work. I'll be able to work on this issue next week.

@manuel-mauky
Copy link
Collaborator

I will merge this PR for now because this way it's easier for others to test the fix.
However, I will keep the issue open until we have added proper tests for this fix.

@manuel-mauky manuel-mauky merged commit 71b47c9 into sialcasa:develop Sep 18, 2018
@manuel-mauky manuel-mauky added this to the 1.8.0 milestone Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants