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

feat: Persist GUI app settings between executions #1169

Merged
merged 6 commits into from
May 27, 2022

Conversation

bdferris-v2
Copy link
Collaborator

Currently, the GUI app does not remember settings (input and output paths, advanced settings) between different executions of the app. This PR adds support for persisting settings between runs of the validator, using the Java Preferences API for storage.

Closes #1165.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues

@maximearmstrong maximearmstrong added this to In Review in The Tech Dashboard (archived) via automation May 27, 2022
Copy link
Contributor

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is fine with the code, but I encounter a strange behavior on macOS. After building the package, I use the .dmg. The application opens then closes automatically, without even displaying the application window. Here are the logs:

May 27, 2022 12:59:38 PM org.mobilitydata.gtfsvalidator.app.gui.Main main
INFOS: gtfs-validator: start
May 27, 2022 12:59:39 PM org.mobilitydata.gtfsvalidator.app.gui.Main main
INFOS: gtfs-validator: exit

Any ideas on what the problem might be?

@bdferris-v2
Copy link
Collaborator Author

The issue was that I hadn't added the "java.prefs" module to the list of required modules in the apps module spec. I fixed that in 09d79fb.

But more broadly, the exception that was thrown was getting added to the run.log because it was an uncaught exception in the UI thread. In 101b2da, I added a custom uncaught exception handler that logs these exceptions (and anything else we missed) into the application log. I've verified that the logging properly logs the missing error above. Doesn't stop the crash but does make it easier to debug.

This all makes me wonder if we can actually run the full application in the CI environment somehow to catch issues like this. Not sure if a GUI application would be supported though...

@maximearmstrong
Copy link
Contributor

This all makes me wonder if we can actually run the full application in the CI environment somehow to catch issues like this. Not sure if a GUI application would be supported though...

If we can do that, that would be great. We could open an issue to investigate it. I don't think it's required for this PR though.

Copy link
Contributor

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The Tech Dashboard (archived) automation moved this from In Review to Approved May 27, 2022
@bdferris-v2 bdferris-v2 merged commit 75c64a9 into MobilityData:master May 27, 2022
The Tech Dashboard (archived) automation moved this from Approved to Done May 27, 2022
@bdferris-v2 bdferris-v2 deleted the issue/1165/persist branch May 27, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Persist GUI app settings between executions
2 participants