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.

Sign up for GitHub

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

Jump to bottom

[hdpowerview] Restructure DTO classes #13630

Merged
merged 4 commits into from
Nov 4, 2022
Merged

[hdpowerview] Restructure DTO classes #13630

merged 4 commits into from
Nov 4, 2022

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Oct 31, 2022 •
edited
Loading

This PR picks up on small cleanup suggested in #13352:

  • Nested DTO classes have been extracted into separate classes.
  • Directory api has been renamed to dto.
  • Test classes are moved into internal to conform with related classes being tested.

Related to: #12678

jlaur added 3 commits October 31, 2022 21:31
@jlaur
Extract nested DTO's to separate classes
17a7d40 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Rename api to dto
e4cb538 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Move test classes into internal
cd7238e 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Copy link
Contributor Author

jlaur commented Oct 31, 2022 •
edited
Loading

@andrewfg - in order to avoid conflicts when rebasing #13355 towards this after a merge, in your branch beforehand:

  • Rename api to dto.
  • Move all tests classes into subdirectory internal.

For reference:

[12678-hdpowerview-gen1-dto-cleanup e4cb538814] Rename api to dto
 32 files changed, 0 insertions(+), 0 deletions(-)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/BatteryKind.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/Color.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/CoordinateSystem.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/Firmware.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/HubFirmware.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/Scene.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/ScheduledEvent.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/ShadeData.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/ShadePosition.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/SurveyData.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/Times.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/UserData.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/RepeaterBlinking.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/RepeaterColor.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/ShadeCalibrate.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/ShadeJog.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/ShadeMotion.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/ShadeMove.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/ShadePositions.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/requests/ShadeStop.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/FirmwareVersion.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/Repeater.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/RepeaterData.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/Repeaters.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/SceneCollections.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/Scenes.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/ScheduledEvents.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/Shade.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/Shades.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/Survey.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/{api => dto}/responses/UserDataResponse.java (100%)

[12678-hdpowerview-gen1-dto-cleanup cd7238e988] Move test classes into internal
 8 files changed, 0 insertions(+), 0 deletions(-)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/AutomationChannelBuilderTest.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/HDPowerViewJUnitTests.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/OnlineCommunicationTest.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/SceneChannelBuilderTest.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/SceneGroupChannelBuilderTest.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/ShadePositionTest.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/providers/MockedLocaleProvider.java (100%)
 rename bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/{ => internal}/providers/MockedTranslationProvider.java (100%)

@jlaur jlaur requested a review from a team October 31, 2022 21:20
andrewfg
andrewfg requested changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

This change causes the HDPowerViewJUnitTests now to fail. The reason is that the test JSON files are located in a relative path from the module path.

@jlaur
Copy link
Contributor Author

jlaur commented Nov 1, 2022

This change causes the HDPowerViewJUnitTests now to fail. The reason is that the test JSON files are located in a relative path from the module path.

I'll have a look at this in the evening, but didn't notice it locally or on build server. Where do you see this?

https://ci.openhab.org/job/PR-openHAB-Addons/12015/console

[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.openhab.binding.hdpowerview.OnlineCommunicationTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.059 s - in org.openhab.binding.hdpowerview.OnlineCommunicationTest
[INFO] Running org.openhab.binding.hdpowerview.SceneChannelBuilderTest
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.766 s - in org.openhab.binding.hdpowerview.SceneChannelBuilderTest
[INFO] Running org.openhab.binding.hdpowerview.HDPowerViewJUnitTests
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.394 s - in org.openhab.binding.hdpowerview.HDPowerViewJUnitTests
[INFO] Running org.openhab.binding.hdpowerview.SceneGroupChannelBuilderTest
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.019 s - in org.openhab.binding.hdpowerview.SceneGroupChannelBuilderTest
[INFO] Running org.openhab.binding.hdpowerview.AutomationChannelBuilderTest
[INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.069 s - in org.openhab.binding.hdpowerview.AutomationChannelBuilderTest
[INFO] Running org.openhab.binding.hdpowerview.ShadePositionTest
[INFO] Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.021 s - in org.openhab.binding.hdpowerview.ShadePositionTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 45, Failures: 0, Errors: 0, Skipped: 0

@jlaur jlaur marked this pull request as draft November 1, 2022 16:19
@jlaur
Finish moving of files and fix namespaces
df8db22 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur marked this pull request as ready for review November 1, 2022 16:37
@jlaur
Copy link
Contributor Author

jlaur commented Nov 1, 2022 •
edited
Loading

I'll have a look at this in the evening, but didn't notice it locally or on build server. Where do you see this?

I don't know how I was able to build and run tests, but I found inconsistencies caused by unsaved changes locally (from refactoring in VS Code). Should be fixed now.

andrewfg
andrewfg approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

LGTM

@jlaur jlaur changed the title [hdpowerview] Generation 1/2 DTO cleanup [hdpowerview] Restructure DTO classes Nov 2, 2022
lolodomo
lolodomo approved these changes Nov 4, 2022
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

@lolodomo lolodomo merged commit c114714 into openhab:main Nov 4, 2022
@lolodomo
Copy link
Contributor

lolodomo commented Nov 4, 2022

@jlaur : I assume you don't want to see that in the release notes ?

@lolodomo lolodomo added this to the 3.4 milestone Nov 4, 2022
@jlaur
Copy link
Contributor Author

jlaur commented Nov 4, 2022

@jlaur : I assume you don't want to see that in the release notes ?

Correct, this is just some renamed files and namespaces without any user impact.

@andrewfg
Copy link
Contributor

andrewfg commented Nov 4, 2022

@jlaur there are still some merge conflicts with the naming adopted in my Gen 3 PR, but I will adapt that PR again to match this one.

andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@jlaur @andan67
[hdpowerview] Restructure DTO classes (openhab#13630)
fb37264 
* Extract nested DTO's to separate classes
* Rename api to dto
* Move test classes into internal
* Finish moving of files and fix namespaces

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@andrewfg andrewfg mentioned this pull request Nov 12, 2022
Merged
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
@jlaur @borazslo
[hdpowerview] Restructure DTO classes (openhab#13630)
1872303 
* Extract nested DTO's to separate classes
* Rename api to dto
* Move test classes into internal
* Finish moving of files and fix namespaces

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@jlaur @psmedley
[hdpowerview] Restructure DTO classes (openhab#13630)
b601795 
* Extract nested DTO's to separate classes
* Rename api to dto
* Move test classes into internal
* Finish moving of files and fix namespaces

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@jlaur @nemerdaud
[hdpowerview] Restructure DTO classes (openhab#13630)
8801841 
* Extract nested DTO's to separate classes
* Rename api to dto
* Move test classes into internal
* Finish moving of files and fix namespaces

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lolodomo lolodomo lolodomo approved these changes

@andrewfg andrewfg andrewfg approved these changes

@andylintner andylintner Awaiting requested review from andylintner andylintner is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
3 participants
@jlaur @lolodomo @andrewfg