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

Replaced window size enum with validated integer #737

Merged
merged 2 commits into from
Sep 26, 2021

Conversation

Mailaender
Copy link
Contributor

Closes #736

@eselmeister
Copy link
Contributor

Are settings validated correctly, that have been persisted already? E.g., if a setting "WIDTH_5" exists already, it needs to be translated to 5, otherwise the process might crash as the existing setting can't be parsed.

@Mailaender
Copy link
Contributor Author

It took me multiple attempts to get backwards compatibility. I went with @JsonDeserialize(using = WindowSizeDeserializer.class) and I exploit the deserialization to always give me a number I can work with. Try using an old process method. It will still run even with legacy settings. Open it up and the editor will tell you to change the value. So this should be a soft update.

@eselmeister
Copy link
Contributor

Does it also work via the Eclipse preference page? In recent time, we have sometimes encountered some problems when underlying values/strategies have changed. Could you create some test cases to ensure the backward compatibility?

@Mailaender Mailaender force-pushed the window-size-enum branch 2 times, most recently from d70f63b to ff1dd9e Compare September 15, 2021 16:04
@Mailaender Mailaender marked this pull request as draft September 16, 2021 09:20
@Mailaender Mailaender marked this pull request as ready for review September 16, 2021 12:52
@Mailaender
Copy link
Contributor Author

Mailaender commented Sep 16, 2021

Alright, this is good to go from my side now. I added a preferences widget that will convert old values on the fly. Process methods will now also work flawlessly and auto-convert old values both during execution and in the UI settings widget. There was a subtle, but hard to find and fix bug in the convoluted settings to UI mapping. It now behaves just like the deserialized process function, in a way that every object has the correct type and custom deserialisation attached to it. It was hard-coded to string conversions before for no apparent reason.

@eselmeister
Copy link
Contributor

Looks good. Some years in the copyright header need adjustments, but I'll take of it this time.

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.

Window Size is stored and displayed as an enum
2 participants