Hi Kenneth, thanks for your interest and your help. It's appreciated.
> fixmes can be safely deleted. Well, of course, they can be safely deleted. It's just a comment. However, they've been added probably because the javadoc of URLUtil.getDomainName(URL url) does not document whether it may return null. A good fix would include to - analyze .getDomainName(...) and find out whether it may return null - update the javadocs accordingly - if null is never returned: remove the unnecessary check for toDomain == null - finally, remove the fixme I'll also comment on your pull request. Thanks, Sebastian On 10/24/2017 10:47 PM, kenneth mcfarland wrote: > Disregard please, after carefully considering the code flow the only thing I > can conclude is the > fixmes can be safely deleted. Foot in mouth. > > On Oct 24, 2017 1:14 PM, "kenneth mcfarland" <[email protected] > <mailto:[email protected]>> wrote: > > Hello, > > I am doing some reading and I believe I have identified a block of code > who's logic is flawed. > I'm on master. > > Go to ParseOutputFormat, I am comparing block of code, block one is line > 362-378 and then block > two is 379 - 394. There are 4 fixemes in these two blocks also, so while > considering what I have > to say it might be worth reading those also. > > Now block one is trying to ignore external links, it seems ok. I am > eyeballing this like my last > fix, doing the logic in my head. My very first question is why we are > doing boolean comparison > for blocks with two different intents with the same variable. > > Explicitly stated, shouldnt line 380 use a boolean of > "ignoreInternalLinks" instead of external? > Aso, there is no exemption filter checking under block two like there is > in block one. > > I know this is abstact, I wish I could just post the code somehow with > snippets but thanks for > taking the time to look as I'm almost dead certain this is flawed logic > again > >
