orchidhq / Orchid

Build and deploy beautiful documentation sites that grow with you
https://orchid.run
GNU General Public License v3.0
518 stars 53 forks source link

Sitemap relative priority #374

Closed vmj closed 4 years ago

vmj commented 4 years ago

The first commit (0ba0493) fixes the bug I wanted to fix.

While I was there, I propose changing the default relativePriority to 0.5. See rationate in the commit (d1b50a1) message. This includes a change in behaviour, so feel free to drop if that is not desired or there are reasons to keep the currect default.

Rest of the commits (eefb486 and 5ca52df) are purely cosmetics. Feel free to drop them if not interested.

I would have written test cases for these but I didn't know how: Kotlin is still read-only for me.

cjbrooks12 commented 4 years ago

The sitemap specification indicates that if <priority> is not given for an entry, it assumes a default value of 0.5, and that's what it was trying to do by only rendering that tag if the value was set. So actually, defaulting that value to 0.5 actually wouldn't necessarily break anything, the priority of pages as search engines index them is 0.5 in either case.

But this actually makes me realize that we should always render that tag regardless of value. If someone intended to give a page a priority of 0.0, then Orchid will not render the tag and search engines will give it a priority of 0.5. Could you make that change too, just remove the if statement around the <priority> tag?

vmj commented 4 years ago

The sitemap specification indicates that if <priority> is not given for an entry, it assumes a default value of 0.5, and that's what it was trying to do by only rendering that tag if the value was set. So actually, defaulting that value to 0.5 actually wouldn't necessarily break anything, the priority of pages as search engines index them is 0.5 in either case.

But this actually makes me realize that we should always render that tag regardless of value. If someone intended to give a page a priority of 0.0, then Orchid will not render the tag and search engines will give it a priority of 0.5. Could you make that change too, just remove the if statement around the <priority> tag?

Changed in 65aa3be.

I actually had if relativePriotity != 0.5 in d1b50a1, which made more sense with the new default, but then accidentally reverted it back to if relativePriority != 0 in eefb486.