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

[Network] Present used arping tool to user #4821

Merged
merged 1 commit into from
Feb 6, 2019
Merged

Conversation

davidgraeff
Copy link
Member

Signed-off-by: davidgraeff [email protected]

Copy link
Member

@martinvw martinvw left a comment

Choose a reason for hiding this comment

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

I added a comment, but feel free to ignore it this time ;-)

arpPingMethod = ArpPingUtilEnum.UNKNOWN_TOOL;
return;
}
arpPingMethod = networkUtils.determineNativeARPpingMethod(arpPingUtilPath);
switch (arpPingMethod) {
case UNKNOWN_TOOL: {
arpPingState = "Unknown arping tool";
Copy link
Member

Choose a reason for hiding this comment

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

You could also store these strings in the enum, then they are closer together, or maybe using a map. This could also work but imho the other options are a little bit more elegant.

Copy link
Member Author

Choose a reason for hiding this comment

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

I might want to apply I18n at some point and thus not hardcoding the strings into the Enum. I do not expect that enum to change a lot, and if it falls through, there is just no name exposed to the user.

@davidgraeff davidgraeff merged commit 9460b18 into master Feb 6, 2019
@davidgraeff davidgraeff deleted the davidgraeff-patch-1 branch February 6, 2019 20:51
@wborn wborn added this to the 2.5 milestone Feb 28, 2019
jannegpriv pushed a commit to jannegpriv/openhab-addons that referenced this pull request Mar 3, 2019
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Jun 19, 2019
ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019
Signed-off-by: davidgraeff <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
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.

3 participants