Add a list of music blog articles #329
Conversation
| fmt.Printf("Failed to query URL: %v\n", err) | ||
| isError = true | ||
| } | ||
| defer resp.Body.Close() |
There was a problem hiding this comment.
resp.Body will be nil if there was an error, so this will panic - you'll need to either goto the error return or do an early-return above (where you set isError = true)
There was a problem hiding this comment.
oop... good spot
There was a problem hiding this comment.
Have pushed a fix if you want to double-check it. Currently throwing to 404 page not ideal but does the job when not configured correctly.
| defer resp.Body.Close() | ||
|
|
||
| // Read the response body | ||
| xmlData, err := io.ReadAll(resp.Body) |
There was a problem hiding this comment.
Might want to look into using the xml Decoder's Decode instead of reading and unmarshaling separately
There was a problem hiding this comment.
Is there a performance benefit to that or is it only about readability?
The XML we're expecting is always limited in size so if it's a performance issue it might not be worth the effort.
There was a problem hiding this comment.
I imagine it's mostly performance since it can decode the XML stream rather than read the whole response into memory, but IMHO the code is marginally cleaner - compare
xmlData, err := io.ReadAll(resp.Body)
if err != nil {
// ...
}
var response SomeType
err = xml.Unmarshal(xmlData, &response)
if err != nil {
// ...
}vs
var response SomeType
err := xml.NewDecoder(resp.Body).Decode(&response)
if err != nil {
// ...
}Your call, I've not got a strong opinion either way.
Co-authored-by: Marks Polakovs <github@markspolakovs.me>
otherwise, yeah, it's fine |
Simple list of articles pulled from mediums RSS feed
Requires one addition to config
Initially open to comment from music team but limited in scope as to what is provided by RSS feed and also I don't want to dump their HTML onto the website.