[Sitemaps] Upgrade Valid / Legal / Strict SitemapUrls #82
Conversation
SitemapUrls can be not valid when they are referenced in a sitemap which it's directory is on a completely different path than the referenced SitemapUrl. All as indicated here: http://www.sitemaps.org/protocol.html#location In order to clarify the validity aspect we need to upgrade the following 1. Add a little more explanations as javadocs and as logs 2. Rename "Legal" (I think only one occurrence) to "valid" (in the parser) 3. Add to the Sitemap class a new method to get all *valid* SitemapUrls 4. When dropping a URL due to invalidity a log should be shown, a URL shouldn't be dropped quietly.
|
|
||
| sb.append("url = \"").append(url).append("\", lastMod = ").append(lastModStr).append(", type = ").append(getType()).append(", processed = ").append(isProcessed()).append(", urlListSize = ") | ||
| .append(urlList.size()); | ||
| sb.append("url = \"").append(url).append("\", lastMod = ").append((getLastModified() == null) ? "null" : SiteMap.getFullDateFormat().format(getLastModified())).append(", type = ") |
There was a problem hiding this comment.
The existing code is easier to read and debug. Inlining it does not add anything, does it?
There was a problem hiding this comment.
I would agree @jnioche. The one line concatenation is almost cryptic.
|
It would also be nice to have a test case which asserts the statement "A SitemapUrl(s) can be not valid when they are referenced in a sitemap with which it's directory is on a completely different path than the referenced SitemapUrl." |
|
The cryptiness comes from our automatic styling plugin we run now in order That ugly line, was styled like all other chained methods, where each In the old style it made sense as I didn't see any added value in those two That being said, now I agree that it is less readable. Will revert it. A. Does it mean that all other code was reviewed ? On Wed, Aug 26, 2015 at 7:54 PM, Lewis John McGibbney <
|
|
Yes, I will push it forward. On Sat, Sep 5, 2015 at 6:39 PM, Lewis John McGibbney <
|
[Sitemaps] Upgrade Valid / Legal / Strict SitemapUrls
|
in Sitemap - why wasn't the line reverted as you said you would? Could you please do that in separate commit? |
|
Didn't forget it, i just hoped the "Merge pull request" on github will give I will change it back in a seperate small commit. On Mon, Sep 7, 2015 at 9:47 PM, Julien Nioche notifications@github.com
|
* Upgraded to Junit v5.5 Updated the annotations and assertions accordingly * Removed unneeded before and after * This is a technical debt Fixing a styling issue I caused about 4 years ago Details can be found here: #82 * Fixed according to @sebastian-nagel code review
Fixes #60
SitemapUrls can be not valid when they are referenced in a sitemap which
it's
directory is on a completely different path than the referenced
SitemapUrl.
All as indicated here:
http://www.sitemaps.org/protocol.html#location
In order to clarify the validity aspect we need to upgrade the following
parser)
shouldn't
be dropped quietly.