anonym wrote (26 Jul 2013 12:49:36 GMT) : > Hi, > branch: test/firewall-check-tag > ticket: Feature #5644 <https://labs.riseup.net/code/issues/5644>
This: > @mac = > REXML::Document.new(@net.xml_desc).elements['network/ip/dhcp/host/'].attributes['mac'] in the Sniffer class, looks like broken encapsulation (see my review of test/reorg for details + a proposal about a similar situation). > - @bridge_name = bridge_name > - @mac = mac > + @vmnet = vmnet > + @net = @vmnet.net > + @bridge_name = @net.bridge_name > + @mac = > REXML::Document.new(@net.xml_desc).elements['network/ip/dhcp/host/'].attributes['mac'] It's not clear to me why @vmnet and @net are added as an instance attributes, since they are apparently not used anywhere else in the class or by its users. Perhaps it's not worth making the class interface & amount of maintained state larger, if all we need in practice is a way for some methods to get the MAC address and bridge name? > def assert_no_leaks Now that this isn't a step definition anymore, but an instance method, I don't think `puts' is the optimal way to report detailed information before throwing an exception. How about storing the detailed text in a local variable, in each of the `if' blocks, and then passing this additional information to `raise'? It seems to me 48a32dd introduces a dependency on @custom_sniffer, while this attribute is only created by the next commit. This breaks bisection. That's all for today :) > This branch depends on test/reorg. Once that other one has its own ticket, #5644 can be marked as blocked by it. Cheers! -- intrigeri | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc _______________________________________________ tails-dev mailing list [email protected] https://mailman.boum.org/listinfo/tails-dev
