josef reidinger napsal(a):
> Michal Zugec napsal(a):
>> ref: refs/heads/network
>> commit 409cd7667168c6d03628ef0cc3b78f1fb280a54f
>> Author: Michal Zugec <[email protected]>
>> Date:   Mon Sep 28 21:56:38 2009 +0200
>>
>>     correctly handle netmask/prefixlen (bnc#539889)
>> ---
>>  .../network/app/controllers/network_controller.rb  |   16 +++++-----------
>>  1 files changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/plugins/network/app/controllers/network_controller.rb 
>> b/plugins/network/app/controllers/network_controller.rb
>> index 65c8f46..8c82060 100644
>> --- a/plugins/network/app/controllers/network_controller.rb
>> +++ b/plugins/network/app/controllers/network_controller.rb
>> @@ -50,26 +50,20 @@ class NetworkController < ApplicationController
>>        ipaddr = "-/-"
>>      end
>>      @ip, @netmask = ipaddr.split "/"
>> -    
>> +    # when detect PREFIXLEN with leading "/"
>> +    @netmask = "/"+...@netmask if ifc.bootproto == "static" && 
>> @netmask.to_i >= 0 && @netmask.to_i <= 32
> ^^^
> I think that this complex condition should be in separated if statement.
> This version is not much readable.
> I suggest use constant for netmask and test range. Somethink like:
> NETMASK_RANGE = 0..32
> if ifc.bootproto == "static" && NETMASK_RANGE.include?(netmask.to_i)
Another solution is
NETMASK_RANGE === netmask.to_i
I don't know which is better readable that we test if number is in interval.
>   @netmask = "/"+...@netmask
> end
> 
>> + 
>>      @name = hn.name
>>      @domain = hn.domain
>>      @nameservers = dns.nameservers
>>      @searchdomains = dns.searches
>>      @default_route = rt.via
>> -    #FIXME: this is ugly and keys are duplicated, but otherwise seems it 
>> doesn't work
>> -    @conf_modes = [["",""], ["static","static"], ["dhcp", "dhcp"]]
>> -    # if unknown item, just add it into list
>> -    found = false
>> -    @conf_modes.each {|a| found=true if a[0] == @conf_mode}
>> -    @conf_modes << [...@conf_mode, @conf_mode] if !found
>> +    @conf_modes = {""=>"", _("static")=>"static", _("dhcp")=>"dhcp"}
>> +    @conf_mod...@conf_mode] ||=...@conf_mode
> 
> Hi, you mix two sollution which I suggest to you and this doesn't work
> For example you have static...translated to cz is staticky and on second
> line you test if exist @conf_modes["static"] but you have
> @conf_mode["staticky"] so this is not produce valid output.
> As I suggest revert this hash (so conf_mode is keys and localized
> strings is values) and in options_for use @conf_modes.invert.
> When I come to work we should look at it together.
>>      
>>    end
>>  
>>  
>> -  # GET /users/1/edit
>> -  def edit
>> -  end
>> -
>>  
>>    # PUT /users/1
>>    # PUT /users/1.xml
> 

-- 
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to