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

[config] Changed 'ConfigDescriptionParameterDTO' field serialization 'defaultValue' -> 'default' #2383

Merged
merged 2 commits into from
May 30, 2021

Conversation

cweitkamp
Copy link
Contributor

  • Changed ConfigDescriptionParameterDTO field serialization: "defaultValue" -> "default"
  • Fixed a minor bug in ConfigDescriptionDTOMapper for field "unit"
  • Added unit tests for ConfigDescriptionDTOMapper, ConfigDescriptionResource and AutomationIntegrationJsonTest

Supersedes #2376
Related to #2148

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

…ault'

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp cweitkamp added enhancement An enhancement or new feature of the Core API breaking labels May 25, 2021
@cweitkamp cweitkamp requested a review from a team as a code owner May 25, 2021 15:15
@@ -29,6 +30,7 @@
public class ConfigDescriptionParameterDTO {

public String context;
@SerializedName(value = "default")
Copy link
Contributor Author

@cweitkamp cweitkamp May 25, 2021

Choose a reason for hiding this comment

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

As mentioned in #2376 (comment) we can use the following syntax to allow both names for parsing.

Suggested change
@SerializedName(value = "default")
@SerializedName(value = "default", alternate = "defaultValue")

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like the safer option in any case. Shall we use this one? What is your preference?

Copy link
Contributor Author

@cweitkamp cweitkamp May 27, 2021

Choose a reason for hiding this comment

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

I'd prefer adding the fallback. We are backwards compatible against parsing then and can remove it after 3.1 has been released.

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Lgtm, thank you!
I'll wait with merging it until after the 3.1.0.M5 build - just to make sure that we do not introduce any regression that we might have overlooked.

@kaikreuzer kaikreuzer merged commit 585e870 into openhab:main May 30, 2021
@cweitkamp cweitkamp deleted the feature-config-dto-default branch May 30, 2021 16:43
ghys pushed a commit to openhab/openhab-webui that referenced this pull request Jun 2, 2021
…alue' -> 'default' (#1077)

Related to openhab/openhab-core#2383.

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@wborn wborn added this to the 3.1 milestone Jun 25, 2021
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/solar-plant-with-battery-widget/130502/8

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
…'defaultValue' -> 'default' (openhab#2383)

* Changed ConfigParameterDTO field serialization 'defaultValue' -> 'default'

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
GitOrigin-RevId: 585e870
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API breaking enhancement An enhancement or new feature of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants