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

[OpenSprinkler] Avoid NPE in case missing config #1888

Merged
merged 1 commit into from
Feb 13, 2017
Merged

[OpenSprinkler] Avoid NPE in case missing config #1888

merged 1 commit into from
Feb 13, 2017

Conversation

marcelrv
Copy link
Contributor

Note. not clear why GH sees the whole class as changed.
Only change made was to move the log message (L42) after the following null check

Signed-off-by: Marcel Verpaalen [email protected]

@mention-bot
Copy link

@marcelrv, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kaikreuzer and @CrackerStealth to be potential reviewers.

@kaikreuzer
Copy link
Member

You mean you didn't change the class OpenSprinklerHTTPHandler at all? Then just revert the change on this file in your commit.

@marcelrv
Copy link
Contributor Author

I intended to change the class wrt to the logger.debug line only.
Eclipse sees only the moved line in the diff, but GH somehow sees all lines in the class as changed.

I checked if it was the indenting, but also that does not appear to be changed

@CrackerStealth
Copy link

It's probably a change in EOL characters from Windows to Unix or Unix to Windows that makes all the lines appear changed.

@kaikreuzer
Copy link
Member

I intended to change the class wrt to the logger.debug line only.

The change you refer to is in the file OpenSprinklerPiHandler, not OpenSprinklerHTTPHandler.
Or did you change the same logging in TWO files?

@CrackerStealth
Copy link

Yeah; the OpenSprinklerHTTPHandler.java file had Windows style line endings on it for some reason; so this change just converted it to Unix style.

A quick compare of the OpenSprinklerHTTPHandler.java shows the same change to the logging line on both files.

CrackerStealth
CrackerStealth approved these changes Feb 13, 2017
View reviewed changes
Copy link

@CrackerStealth CrackerStealth left a comment

Choose a reason for hiding this comment

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

Changes look fine to me to handle the case that config does not exist and DEBUG logging is enabled.

@kaikreuzer
Copy link
Member

Ok, thanks @CrackerStealth & @marcelrv!

@kaikreuzer kaikreuzer merged commit 89aca4c into openhab:master Feb 13, 2017
@marcelrv marcelrv deleted the Osprink-npe branch February 16, 2017 14:40
fharni pushed a commit to fharni/openhab2-addons that referenced this pull request Feb 19, 2017
@marcelrv @fharni
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
4116e30 
Signed-off-by: Marcel Verpaalen <[email protected]>
jarlebh pushed a commit to jarlebh/openhab2-addons that referenced this pull request Mar 4, 2017
@marcelrv @jarlebh
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
f25dacf 
Signed-off-by: Marcel Verpaalen <[email protected]>
cweitkamp pushed a commit to cweitkamp/openhab-addons that referenced this pull request Mar 26, 2017
@marcelrv @cweitkamp
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
f4f169f 
Signed-off-by: Marcel Verpaalen <[email protected]>
tratho pushed a commit to tratho/openhab2-addons that referenced this pull request May 5, 2017
@marcelrv @tratho
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
1d5645f 
Signed-off-by: Marcel Verpaalen <[email protected]>
Markinus pushed a commit to Markinus/openhab2-addons that referenced this pull request Jul 2, 2017
@marcelrv @Markinus
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
b8aedd1 
Signed-off-by: Marcel Verpaalen <[email protected]>
aogorek pushed a commit to aogorek/openhab2-addons that referenced this pull request Jul 5, 2017
@marcelrv
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
5bf2f32 
Signed-off-by: Marcel Verpaalen <[email protected]>
Markinus pushed a commit to Markinus/openhab2-addons that referenced this pull request Sep 8, 2017
@marcelrv @Markinus
[OpenSprinkler] Avoid NPE in case missing config (openhab#1888)
5cbd3b0 
Signed-off-by: Marcel Verpaalen <[email protected]>
Flole998 pushed a commit to Flole998/openhab-addons that referenced this pull request Dec 30, 2021
@clinique
[OAuth] Added capability for custom deserializer (openhab#1891)
f7e0339 
* Added capability for custom deserializer

Closes openhab#1888

Signed-off-by: clinique <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Aug 12, 2023
@stefan-hoehn @Confectrician
new menu entry settings (openhab#1888)
c675bae 
* new menu entry settings

Signed-off-by: Stefan Höhn <[email protected]>

* Apply suggestions from code review

Signed-off-by: Jerome Luckenbach <[email protected]>

* Apply suggestions from code review

Signed-off-by: Jerome Luckenbach <[email protected]>

* Markdown fixes from code review

Signed-off-by: Jerome Luckenbach <[email protected]>

* Markdown fixes from code review (Round 2)

Signed-off-by: Jerome Luckenbach <[email protected]>

Signed-off-by: Stefan Höhn <[email protected]>
Signed-off-by: Jerome Luckenbach <[email protected]>
Co-authored-by: Jerome Luckenbach <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CrackerStealth CrackerStealth CrackerStealth approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@marcelrv @mention-bot @kaikreuzer @CrackerStealth