Skip to content

Support for sitemaps #4261#5084

Merged
kcondon merged 22 commits intodevelopfrom
4261-sitemap
Oct 4, 2018
Merged

Support for sitemaps #4261#5084
kcondon merged 22 commits intodevelopfrom
4261-sitemap

Conversation

@pdurbin
Copy link
Member

@pdurbin pdurbin commented Sep 24, 2018

connects to #4261

@pdurbin pdurbin changed the title add doc stub for sitemaps #4261 Support for sitemaps #4261 Sep 24, 2018
@coveralls
Copy link

coveralls commented Sep 28, 2018

Coverage Status

Coverage increased (+0.2%) to 15.654% when pulling c80dc43 on 4261-sitemap into 0b25f9d on develop.

Copy link
Contributor

@matthew-a-dunlap matthew-a-dunlap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall!

I have a question about validation in the comments below.

I deleted my comment on descriptive information in sitemaps because I think that's only for hard to parse content (video/images).

}
sitemapPathAndFile = sitemapPath + File.separator + SITEMAP_FILENAME;

DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this generation could benefit from schema validation via DocumentBuilderFactory.setSchema(schema) ?


Exception notValidAgainstSchemaException = null;
try {
assertTrue(XmlValidator.validateXmlSchema("/tmp/sitemap.xml", new URL("https://www.sitemaps.org/schemas/sitemap/0.9/sitemap.xsd")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I see we validate here. Do we know if-when google updates there schema whether they will support old versions? I could see value in general of validating against the schema on each run if it lets us know more quickly that google won't stop indexing us.

Copy link
Contributor

@matthew-a-dunlap matthew-a-dunlap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After talking with Phil I'm fine with the code as it stands

@kcondon kcondon merged commit eed3ac1 into develop Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants