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

Set LAN interface even if none exist #297

Merged
merged 1 commit into from Oct 3, 2018
Merged

Conversation

asoltys
Copy link
Contributor

@asoltys asoltys commented Oct 1, 2018

Fixes an issue where if you had removed all LAN interfaces you wouldn't be
able to re-add one since the network.lan.ifname setting wouldn't exist and the
code would return an Err in that case.

Now if the setting doesn't exist, we go ahead and set it instead of returning
an Err.

Copy link
Contributor

@drozdziak1 drozdziak1 left a comment

Choose a reason for hiding this comment

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

^^^

@drozdziak1
Copy link
Contributor

@asoltys Sounds good, I'd appreciate if you could adjust this PR for that. Thanks!

@asoltys
Copy link
Contributor Author

asoltys commented Oct 3, 2018

I moved the string parsing into the Kernel but decided that rather than adding a new error type to return it makes more sense to just return Ok() with an empty string if the uci setting wasn't found.

Copy link
Member

@jkilpatr jkilpatr left a comment

Choose a reason for hiding this comment

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

I have some conceptual problems with get uci var modifying UCI. why not check for the error at call time?

@jkilpatr
Copy link
Member

jkilpatr commented Oct 3, 2018

also why where there three commits that undo each other? Just cherry pick the second one and lets go for it.

@asoltys
Copy link
Contributor Author

asoltys commented Oct 3, 2018

Ok, rebased

@asoltys asoltys dismissed drozdziak1’s stale review October 3, 2018 21:02

@jkilpatr said lets just keep the error checking in the calling code

@jkilpatr jkilpatr merged commit aa23946 into althea-net:master Oct 3, 2018
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.

None yet

3 participants