-
Notifications
You must be signed in to change notification settings - Fork 80
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
Option Static routes support - RFC 3442 #43
Option Static routes support - RFC 3442 #43
Conversation
@@ -34,6 +34,11 @@ option fqdn.no-client-update on; # set the "O" and "S" flag bits | |||
option fqdn.rcode2 255; | |||
option pxegrub code 150 = text ; | |||
|
|||
<% if has_variable?( 'option_static_route' ) -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you also need a check if it's non-empty.
Would you mind adding a test case as well? And the tests need to pass. Otherwise I think it's a good addition. |
Travis seems to be happy :) I hope the tests are ok for you. |
I don't use the -dhcp module, so I can't say too much content-wise, but the tests look OK, so 👍 @davidblaisonneau-orange could you squash your commits together into one? |
@davidblaisonneau-orange there are still 8 commits. |
BTW, I don't mind squashing it and pushing the squashed commit if you can't figure it out. |
This should be better. Sorry for the delay, PR is something quite new for me |
@davidblaisonneau-orange no problem, we've all been there. The only thing that would finish it if you could fix the commit message to something more meaningful. |
Now it should be fine :) |
👍 |
@@ -16,6 +16,12 @@ subnet <%= @network %> netmask <%= @mask %> { | |||
<% if @gateway && !@gateway.strip.empty? -%> | |||
option routers <%= @gateway %>; | |||
<% end -%> | |||
<% if @static_routes | |||
@static_routes.each do |static_route| -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please use
<% if @static_routes -%>
<% @static_routes.each do |static_route| -%>
...
<% end -%>
<% end -%>
or similar here? I was just confused by seeing two <% end -%>
s and thought of a duplicate line.
👍 after fixing the minor whitespace nit. |
Done |
👍 |
Thanks! |
Thanks to you ! |
This PR add an option to add static routes with DHCP.
See: