Hey Toad, Thanks for your points. Matthew Toseland <[email protected]> Okay, one or two things to deal with first: > - It would be better to just say which branch you want us to > merge. There is a button for this on github, otherwise post a > link. (The link you posted in the first commit was wrong) I'm sorry about the link. My recollection was that I even tested the link and it was pointing to the right commit. Maybe github doesn't do permanent link.
> - You should always use git diff -u --ignore-space-change So, If I'm posting a branch instead of a patch, I don't need to do that, Freeneters are going to take care of that, Correct? > - Calling reportMTU inside the loop is a bit odd, but I guess it's > unavoidable? I believe IPv4 and IPv6 on the same interface is how it > will generally work, in which case the JVM should report whichever MTU > is lower? This should be mentioned in the comments anyway. Yes that was odd for me too. My understanding is that the interface get max MTU independent of which ip protocol is going to be used. Then when the connection is establish the MTU will be different based on if it is a IPv6 or IPv4 connection and who is on the way. Further more, the MTU is the maximum of any MTU (where it is IPv6 or IPv4). So it can't be minimum of both MTU (unless they are equal). Because the way to do MTU discovery is to set MTU equal to the interface's MTU and decrease till eventually it passes through the connection at that time you know you found the MTU of that specific connection. So you need to decrease it to get the IPv? MTU. So, if what I said makes sense then probably the reportMTU does not need the IPv6 argument and does not need to be called in the loop. > - Freenet typically uses a style of 4 space tabs and 100 char lines > IIRC. So you are chopping off some lines unnecessary. Hmmm, I thought > there was a page for this on the wiki? Apparently not ... :| Yes, I was confused and I looked into the "Get involved" page and I didn't find anything. I guess a wiki page with a link there would be good. Clearly the files that I edited where using TAB char instead of 4xSpaces (I guess your editor is representing TAB with 4 spaces so you don't see the difference). But most recent development hygienes recommend Spaces instead of TAB as you said. So, I'm going to stick with your advice (4 spaces indentation). I thought the line limits was 80 and TAB was represented as 8 Char here. I'll change them to 100 and change TAB to be represented by 4 Char (but not use it). > - Once this is deployed we should update the plugin as well... Indeed. basically it should not compute the MTU, cause we do it in checkpoint I guess. Am I correct? Thanks, Vmon >> >> Thanks, >> Vmon >> >> [email protected] writes: >> >> > Still need to ignore local ips but that shouldn't be hard. I'll add that >> > shortly. Also it calls updateMTU more frequently. I don't know if it is >> > a deal breaker. >> > >> > Anyway, I ran it and at least I was able to browse normally so I haven't >> > broke anything. >> > >> > https://github.com/vmon/fred-staging/commit/1b108d0c0210731c5424af0e36415a867004b5c1 >> > >> > Cheers, >> > Vmon _______________________________________________ Devl mailing list [email protected] https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
