Integration Tests and handling for alternates and canonical #7
Conversation
| let mut unique_hreflangs = HashSet::new(); | ||
| for alternate in &alternates { | ||
| if !unique_hreflangs.insert(&alternate.hreflang) { | ||
| return Err(UrlError::DuplicateAlternateHreflangs( |
There was a problem hiding this comment.
Instead of manually checking for uniqueness within a Vec, could pub alternates: Option<Vec<Alternate>>, be refactored to be pub alternates: Option<Set<Alternate>>, from the beginning?
| @@ -100,6 +113,22 @@ impl Url { | |||
| } | |||
| }; | |||
|
|
|||
| let alternates: Option<Vec<Alternate>> = match alternates { | |||
| None => None, | |||
| Some(alternates) => { | |||
There was a problem hiding this comment.
In this Some(alternates) condition, I like to check if alternates.is_empty() and return None if so. This helps limit the total number of ways to represent "I have zero alternates".
|
Thanks for writing these great integration tests! |
|
Sorry it took me so long to take another look at this. I will make the according changes in the next weeks. |
|
Take your time - I appreciate your work! |
| self | ||
| } | ||
|
|
||
| pub fn push_alternate(&mut self, hreflang: String, href: String) -> &mut Self { |
There was a problem hiding this comment.
Can you use impl ToString instead of String for hreflang and href here? This makes for a nicer interface.
I have added the ability to specify language alternates and the canonical URL, so that they can be included in the sitemap.
I have also added integration tests for generating the sitemaps.
Please review the changes made and notify me if anything doesn't fit correctly. I'll be happy to make any necessary adjustments accordingly.