Implement Display for Changelog. Hooked up to parsing validation. #595
Conversation
Next up is implement all the validation for 'ci'.
…ake sure input string == output string.
| @@ -29,6 +30,16 @@ pub enum ReleaseType { | |||
| Patch, | |||
| } | |||
|
|
|||
| impl Display for ReleaseType { | |||
There was a problem hiding this comment.
Unrelated but I think it's worth doing it soon, is to rename ReleaseType to Scope to be a bit more abstract, because this type is used for 2 things:
- Describe the scope or impact of a change.
- Describe the highest scope of a change between 2 releases.
And the second thing is actually only a consequence of the first, so the name should be more relevant to the first, which is describing the scope/impact of a change.
There was a problem hiding this comment.
Actually, we could also use the name Severity since it's used here.
|
|
||
| ensure!( | ||
| output_string == contents, | ||
| "Input string does not equal rendered changelog.\n\nIn: {contents}\n\nOut: {contents}" |
There was a problem hiding this comment.
It's probably more readable to use the similar-asserts crate like here. Use the same version as the wire-derive crate does. Ideally we should have a test for that, but it's not there yet.
|
|
||
| impl Display for Changelog { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| let releases_string = self.releases.iter().map(|release| format!("{release}")).join("\n\n"); |
There was a problem hiding this comment.
This is not efficient to build a string for each version and then join them. We want to output directly to the formatter.
| let releases_string = self.releases.iter().map(|release| format!("{release}")).join("\n\n"); | |
| for release in &self.releases { | |
| writeln!(f, "{release}\n")?; | |
| } |
Also this permits getting rid of itertools which is usually not useful.
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| let releases_string = self.releases.iter().map(|release| format!("{release}")).join("\n\n"); | ||
|
|
||
| write!( |
There was a problem hiding this comment.
Same, we can split this in 2 around the for-loop above.
writeln!(f, "# Changelog\n")?;
for release in &self.releases { ... }
writeln!(f, "<!-- Increment ...: {skip_counter} -->")| let description_string = | ||
| descriptions.iter().map(|description| format!("- {description}")).join("\n"); | ||
|
|
||
| write!(f, "\n\n### {release_type}\n\n{description_string}")?; |
There was a problem hiding this comment.
Same here.
| let description_string = | |
| descriptions.iter().map(|description| format!("- {description}")).join("\n"); | |
| write!(f, "\n\n### {release_type}\n\n{description_string}")?; | |
| writeln!(f, "### {scope}\n")?; | |
| for description in descriptions { | |
| writeln!(f, "- {description}")?; | |
| } |
|
By the way, there will be merge conflicts due to #601 when we'll merge into main. We'll have to decide how to do that since GitHub doesn't support reviewing merge commits. |
Diff will clean up after PR #584 is merged