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

[meteoalerte] Météo Alerte binding #7200

Merged
merged 16 commits into from
May 30, 2020
Merged

[meteoalerte] Météo Alerte binding #7200

merged 16 commits into from
May 30, 2020

Conversation

clinique
Copy link
Contributor

No description provided.

@lolodomo
Copy link
Contributor

It looks like your README file si fully irrelevant, probably a copy from another binding ?

@clinique
Copy link
Contributor Author

It looks like your README file si fully irrelevant, probably a copy from another binding ?

Yes, I did not work on it at all as of now

@lolodomo
Copy link
Contributor

With the code and the URL in the code, I see what is this binding.

@openhab openhab deleted a comment from TravisBuddy Mar 21, 2020
@clinique
Copy link
Contributor Author

With the code and the URL in the code, I see what is this binding.

You can test it ASAYW, it is functional.

@openhab openhab deleted a comment from TravisBuddy Mar 21, 2020
@openhab openhab deleted a comment from TravisBuddy Mar 22, 2020
@openhab openhab deleted a comment from TravisBuddy Mar 22, 2020
@wborn wborn added new binding If someone has started to work on a binding. For a new binding PR. work in progress A PR that is not yet ready to be merged labels Mar 29, 2020
@clinique clinique requested a review from a team as a code owner April 28, 2020 13:59
@clinique clinique changed the title [WIP] Météo Alerte binding Météo Alerte binding Apr 28, 2020
@clinique clinique removed the work in progress A PR that is not yet ready to be merged label Apr 28, 2020
@openhab openhab deleted a comment from TravisBuddy Apr 28, 2020
@clinique
Correction pom file
0b8fade 
Signed-off-by: clinique <[email protected]>
@openhab openhab deleted a comment from TravisBuddy Apr 28, 2020
@clinique
DTO cleaning
ff165fd 
Signed-off-by: clinique <[email protected]>
@openhab openhab deleted a comment from TravisBuddy Apr 28, 2020
@clinique clinique requested review from cpmeister, J-N-K and wborn April 29, 2020 08:32
@clinique
Some leftovers
95c824c 
Signed-off-by: clinique <[email protected]>
@TravisBuddy
Copy link

Travis tests were successful

Hey @clinique,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@clinique clinique requested a review from cpmeister May 24, 2020 07:55
@openhab openhab deleted a comment from TravisBuddy May 24, 2020
@cpmeister cpmeister added the cre Coordinated Review Effort label May 25, 2020
@openhab openhab deleted a comment from TravisBuddy May 25, 2020
J-N-K
J-N-K requested changes May 26, 2020
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

Thanks, only some minor comments. I'm a bit unsure about the french channel names and description, but I'm ok since I think that this will mostly be used by people who speak french.

bundles/org.openhab.binding.meteoalerte/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.meteoalerte/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.meteoalerte/README.md Show resolved Hide resolved
bundles/org.openhab.binding.meteoalerte/README.md Outdated Show resolved Hide resolved
...lerte/src/main/java/org/openhab/binding/meteoalerte/internal/handler/MeteoAlerteHandler.java Outdated Show resolved Hide resolved
...lerte/src/main/java/org/openhab/binding/meteoalerte/internal/handler/MeteoAlerteHandler.java Outdated Show resolved Hide resolved
...lerte/src/main/java/org/openhab/binding/meteoalerte/internal/handler/MeteoAlerteHandler.java Outdated Show resolved Hide resolved
...lerte/src/main/java/org/openhab/binding/meteoalerte/internal/handler/MeteoAlerteHandler.java Outdated Show resolved Hide resolved
...lerte/src/main/java/org/openhab/binding/meteoalerte/internal/handler/MeteoAlerteHandler.java Show resolved Hide resolved
....binding.meteoalerte/src/main/java/org/openhab/binding/meteoalerte/internal/json/Fields.java Outdated
* @author Gaël L'hopital - Initial contribution
*/
@NonNullByDefault
public class Fields {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a name that is not so close to a standard java class name (Field). Maybe ResponseFieldDTO, WDYT?

lolodomo
lolodomo requested changes May 27, 2020
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.

One wrong constant value

...oalerte/src/main/java/org/openhab/binding/meteoalerte/internal/MeteoAlerteConfiguration.java Outdated
@@ -23,5 +23,5 @@
@NonNullByDefault
public class MeteoAlerteConfiguration {
public String department = "";
public Integer refresh = 24;
public Integer refresh = 1400;
Copy link
Contributor

Choose a reason for hiding this comment

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

1440

@TravisBuddy
Copy link

Travis tests were successful

Hey @clinique,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@clinique
Typo
303cb20 
Signed-off-by: clinique <[email protected]>
@TravisBuddy
Copy link

Travis tests were successful

Hey @clinique,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

lolodomo
lolodomo approved these changes May 27, 2020
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

@J-N-K J-N-K removed the request for review from wborn May 30, 2020 15:38
@J-N-K
Copy link
Member

J-N-K commented May 30, 2020

Jenkins fails due to different POM-version. Checked ok.

@J-N-K J-N-K merged commit eb074e7 into openhab:2.5.x May 30, 2020
@J-N-K J-N-K removed the cre Coordinated Review Effort label May 30, 2020
@J-N-K J-N-K added this to the 2.5.6 milestone May 30, 2020
Dries-Vandenneucker added a commit to Dries-Vandenneucker/openhab-addons that referenced this pull request May 31, 2020
@Dries-Vandenneucker
Merge branch '2.5.x' into StiebelBinding
b77c7cd 
* 2.5.x: (174 commits)
  [hpprinter] Add additional data points and refactoring (openhab#7805)
  [neohub] new/legacy API; null annotations; enhancements; bugs; logging (openhab#7767)
  [meteoalerte] Initial contribution (openhab#7200)
  [lgwebos] Console command to show the access key (openhab#7801)
  [hue] Refactored state handling and fix polling after command (openhab#7518)
  [telegram] add attachment URL (openhab#7816)
  [siemensrds] readme adjusted to match openhab#7814 (openhab#7819)
  [lametrictime] correctly parse response (openhab#7818)
  [Seneye] Bug fix for using Pond or Home sensors. (openhab#7797)
  [siemensrds] apply UoM quantityType percent for relative humidity (openhab#7814)
  [alarmdecoder] Add vzone thing for virtual zone control (openhab#7800)
  [hue] Channel alert added for groups (openhab#7810)
  [hue] Keep compatibility with hue emulation for groups (openhab#7809)
  [dscalarm] Bridge/things management refactored (openhab#7748)
  [avmfritz] Add link to Fensterkontakt (magnetisch) to docs (openhab#7806)
  [deconz] add light/blind support and additional sensors (openhab#7608)
  [homekit] add support for min/max values for temperature (openhab#7782)
  [tesla] Use CXF JAX-RS client builder, if available (openhab#7804)
  [mqtt.homie] Improve Homie discovery time (openhab#7760)
  [siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging (openhab#7769)
  ...
@clinique clinique deleted the MeteoAlerte branch June 5, 2020 11:14
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
@clinique @J-N-K
[meteoalerte] Initial contribution (openhab#7200)
2463cda 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
@clinique @CSchlipp
[meteoalerte] Initial contribution (openhab#7200)
b1a6da4 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
Signed-off-by: CSchlipp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@clinique @andrewfg
[meteoalerte] Initial contribution (openhab#7200)
a37a1ad 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@clinique @andrewfg
[meteoalerte] Initial contribution (openhab#7200)
c676172 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@clinique @andrewfg
[meteoalerte] Initial contribution (openhab#7200)
ef867e1 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
@clinique @andrewfg
[meteoalerte] Initial contribution (openhab#7200)
d401672 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
@clinique @DaanMeijer
[meteoalerte] Initial contribution (openhab#7200)
02885c5 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
@clinique @markus7017
[meteoalerte] Initial contribution (openhab#7200)
7b076f0 
* Initial commit of Mété Alerte binding

Signed-off-by: clinique <[email protected]>
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

@J-N-K J-N-K J-N-K approved these changes

@cpmeister cpmeister cpmeister approved these changes

Assignees
No one assigned
Labels
new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Milestone
2.5.6
Development

Successfully merging this pull request may close these issues.
6 participants
@clinique @lolodomo @TravisBuddy @J-N-K @cpmeister @wborn