Unverified Commit 094924ca authored by Harry Barber's avatar Harry Barber Committed by GitHub
Browse files

Add CHANGELOG support for multiple reference authors (#2012) 



## Motivation and Context

Currently only a single author can be provided to a changelog entry.
This does not support the cases where a single PR was coauthored by
multiple authors and/or multiple PRs were authored by different
individuals.

## Description

- Accept a list of strings, in addition to a single string in the
`author` field. Each author in the list is added as a contributor to
every PR provided in references.
- Accept references of the form `{ "id": <ID>, "authors": <Author> }`,
in addition to `ID`. Each author given in `authors` is added as a
contributor to that PR. Authors common to all references will be
serialized in the same way as using the top-level `author` field.

## Notes

- This should not be a breaking change. Existing `CHANGELOG.next.toml`
should be deserialized and then serialized identically.

---------

Co-authored-by: default avatarHarry Barber <hlbarber@amazon.co.uk>
Co-authored-by: default avatarJohn DiSanti <jdisanti@amazon.com>
parent d42727e3

tools/ci-build/changelogger/src/render.rs

+23 −12
Original line number Original line Diff line number Diff line
@@ -199,9 +199,13 @@ fn render_entry(entry: &HandAuthoredEntry, mut out: &mut String) { 
        .map(|t| t.to_string())

        .map(|t| t.to_string())

        .chain(entry.references.iter().map(to_md_link))

        .chain(entry.references.iter().map(to_md_link))

        .collect::<Vec<_>>();

        .collect::<Vec<_>>();

    if !is_maintainer(&entry.author) {

    let non_maintainers = entry

        references.push(format!("@{}", entry.author));

        .authors

    };

        .iter()

        .filter(|author| !is_maintainer(author))

        .map(|author| format!("@{author}"));

    references.extend(non_maintainers);



    if !references.is_empty() {

    if !references.is_empty() {

        write!(meta, "({}) ", references.join(", ")).unwrap();

        write!(meta, "({}) ", references.join(", ")).unwrap();

    }

    }
@@ -372,7 +376,8 @@ fn render_sdk_model_entries<'a>( 
fn render_external_contributors(entries: &[ChangelogEntry], out: &mut String) {

fn render_external_contributors(entries: &[ChangelogEntry], out: &mut String) {

    let mut external_contribs = entries

    let mut external_contribs = entries

        .iter()

        .iter()

        .filter_map(|entry| entry.hand_authored().map(|e| &e.author))

        .filter_map(|entry| entry.hand_authored().map(|e| &e.authors))

        .flat_map(|authors| authors.iter())

        .filter(|author| !is_maintainer(author))

        .filter(|author| !is_maintainer(author))

        .collect::<Vec<_>>();

        .collect::<Vec<_>>();

    if external_contribs.is_empty() {

    if external_contribs.is_empty() {
@@ -388,7 +393,11 @@ fn render_external_contributors(entries: &[ChangelogEntry], out: &mut String) { 
            .filter(|entry| {

            .filter(|entry| {

                entry

                entry

                    .hand_authored()

                    .hand_authored()

                    .map(|e| e.author.eq_ignore_ascii_case(contributor_handle.as_str()))

                    .map(|e| {

                        e.authors

                            .iter()

                            .any(|author| author.eq_ignore_ascii_case(contributor_handle.as_str()))

                    })

                    .unwrap_or(false)

                    .unwrap_or(false)

            })

            })

            .flat_map(|entry| {

            .flat_map(|entry| {
@@ -475,6 +484,7 @@ fn render( 
        entries.iter().filter_map(ChangelogEntry::aws_sdk_model),

        entries.iter().filter_map(ChangelogEntry::aws_sdk_model),

        &mut out,

        &mut out,

    );

    );



    render_external_contributors(entries, &mut out);

    render_external_contributors(entries, &mut out);

    render_crate_versions(crate_version_metadata_map, &mut out);

    render_crate_versions(crate_version_metadata_map, &mut out);
@@ -500,13 +510,13 @@ mod test { 
    fn end_to_end_changelog() {

    fn end_to_end_changelog() {

        let changelog_toml = r#"

        let changelog_toml = r#"

[[smithy-rs]]

[[smithy-rs]]

author = "rcoh"

author = ["rcoh", "jdisanti"]

message = "I made a major change to update the code generator"

message = "I made a major change to update the code generator"

meta = { breaking = true, tada = false, bug = false }

meta = { breaking = true, tada = false, bug = false }

references = ["smithy-rs#445"]

references = ["smithy-rs#445"]





[[smithy-rs]]

[[smithy-rs]]

author = "external-contrib"

author = ["external-contrib", "other-external-dev"]

message = "I made a change to update the code generator"

message = "I made a change to update the code generator"

meta = { breaking = false, tada = true, bug = false }

meta = { breaking = false, tada = true, bug = false }

references = ["smithy-rs#446", "aws-sdk#123"]

references = ["smithy-rs#446", "aws-sdk#123"]
@@ -530,7 +540,7 @@ meta = { breaking = false, tada = true, bug = false } 
references = ["smithy-rs#446"]

references = ["smithy-rs#446"]





[[smithy-rs]]

[[smithy-rs]]

author = "external-contrib"

authors = ["external-contrib", "other-external-dev"]

message = """

message = """

I made a change to update the code generator

I made a change to update the code generator
@@ -538,7 +548,7 @@ I made a change to update the code generator 
blah blah

blah blah

"""

"""

meta = { breaking = false, tada = true, bug = false }

meta = { breaking = false, tada = true, bug = false }

references = ["smithy-rs#446"]

references = ["smithy-rs#446", "smithy-rs#447"]





[[aws-sdk-model]]

[[aws-sdk-model]]

module = "aws-sdk-s3"

module = "aws-sdk-s3"
@@ -572,8 +582,8 @@ v0.3.0 (January 4th, 2022) 
- :warning: (all, [smithy-rs#445](https://github.com/smithy-lang/smithy-rs/issues/445)) I made a major change to update the code generator

- :warning: (all, [smithy-rs#445](https://github.com/smithy-lang/smithy-rs/issues/445)) I made a major change to update the code generator





**New this release:**

**New this release:**

- :tada: (all, [smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446), [aws-sdk#123](https://github.com/aws/aws-sdk/issues/123), @external-contrib) I made a change to update the code generator

- :tada: (all, [smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446), [aws-sdk#123](https://github.com/aws/aws-sdk/issues/123), @external-contrib, @other-external-dev) I made a change to update the code generator

- :tada: (all, [smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446), @external-contrib) I made a change to update the code generator

- :tada: (all, [smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446), [smithy-rs#447](https://github.com/smithy-lang/smithy-rs/issues/447), @external-contrib, @other-external-dev) I made a change to update the code generator





    **Update guide:**

    **Update guide:**

    blah blah

    blah blah
@@ -582,7 +592,8 @@ v0.3.0 (January 4th, 2022) 
**Contributors**

**Contributors**

Thank you for your contributions! ❤

Thank you for your contributions! ❤

- @another-contrib ([smithy-rs#200](https://github.com/smithy-lang/smithy-rs/issues/200))

- @another-contrib ([smithy-rs#200](https://github.com/smithy-lang/smithy-rs/issues/200))

- @external-contrib ([smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446))

- @external-contrib ([smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446), [smithy-rs#447](https://github.com/smithy-lang/smithy-rs/issues/447))

- @other-external-dev ([smithy-rs#446](https://github.com/smithy-lang/smithy-rs/issues/446), [smithy-rs#447](https://github.com/smithy-lang/smithy-rs/issues/447))





"#

"#

        .trim_start();

        .trim_start();

tools/ci-build/smithy-rs-tool-common/src/changelog.rs

+94 −4
Original line number Original line Diff line number Diff line
@@ -120,11 +120,68 @@ impl FromStr for Reference { 
    }

    }

}

}





#[derive(Deserialize, Serialize)]

#[serde(untagged)]

enum AuthorsInner {

    Single(String),

    Multiple(Vec<String>),

}



#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]

#[serde(from = "AuthorsInner", into = "AuthorsInner")]

pub struct Authors(pub(super) Vec<String>);



impl From<AuthorsInner> for Authors {

    fn from(value: AuthorsInner) -> Self {

        match value {

            AuthorsInner::Single(author) => Authors(vec![author]),

            AuthorsInner::Multiple(authors) => Authors(authors),

        }

    }

}



impl From<Authors> for AuthorsInner {

    fn from(mut value: Authors) -> Self {

        match value.0.len() {

            0 => Self::Single("".to_string()),

            1 => Self::Single(value.0.pop().unwrap()),

            _ => Self::Multiple(value.0),

        }

    }

}



impl Authors {

    pub fn iter(&self) -> impl Iterator<Item = &String> {

        self.0.iter()

    }



    // Checks whether the number of authors is 0 or any author has a empty name.

    pub fn is_empty(&self) -> bool {

        self.0.is_empty() || self.iter().any(String::is_empty)

    }



    pub fn validate_usernames(&self) -> Result<()> {

        fn validate_username(author: &str) -> Result<()> {

            if !author.chars().all(|c| c.is_alphanumeric() || c == '-') {

                bail!("Author, \"{author}\", is not a valid GitHub username: [a-zA-Z0-9\\-]")

            }

            Ok(())

        }

        for author in self.iter() {

            validate_username(author)?

        }

        Ok(())

    }

}



#[derive(Clone, Debug, Default, Deserialize, Serialize)]

#[derive(Clone, Debug, Default, Deserialize, Serialize)]

pub struct HandAuthoredEntry {

pub struct HandAuthoredEntry {

    pub message: String,

    pub message: String,

    pub meta: Meta,

    pub meta: Meta,

    pub author: String,

    // Retain singular field named "author" for backwards compatibility,

    // but also accept plural "authors".

    #[serde(rename = "author", alias = "authors")]

    pub authors: Authors,

    #[serde(default)]

    #[serde(default)]

    pub references: Vec<Reference>,

    pub references: Vec<Reference>,

    /// Optional commit hash to indicate "since when" these changes were made

    /// Optional commit hash to indicate "since when" these changes were made
@@ -140,10 +197,14 @@ pub struct HandAuthoredEntry { 
impl HandAuthoredEntry {

impl HandAuthoredEntry {

    /// Validate a changelog entry to ensure it follows standards

    /// Validate a changelog entry to ensure it follows standards

    pub fn validate(&self, validation_set: ValidationSet) -> Result<()> {

    pub fn validate(&self, validation_set: ValidationSet) -> Result<()> {

        if self.author.is_empty() {

        if self.authors.iter().any(|author| author.trim().is_empty()) {

            bail!("Author must be set (was empty)");

            bail!("Author must be set (was empty)");

        }

        }

        if !self.author.chars().all(|c| c.is_alphanumeric() || c == '-') {

        if !self

            .authors

            .iter()

            .any(|author| author.chars().all(|c| c.is_alphanumeric() || c == '-'))

        {

            bail!("Author must be valid GitHub username: [a-zA-Z0-9\\-]")

            bail!("Author must be valid GitHub username: [a-zA-Z0-9\\-]")

        }

        }

        if validation_set == ValidationSet::Development && self.references.is_empty() {

        if validation_set == ValidationSet::Development && self.references.is_empty() {
@@ -291,7 +352,7 @@ impl Changelog { 




#[cfg(test)]

#[cfg(test)]

mod tests {

mod tests {

    use super::{Changelog, HandAuthoredEntry, SdkAffected, ValidationSet};

    use super::*;

    use anyhow::Context;

    use anyhow::Context;





    #[test]

    #[test]
@@ -520,5 +581,34 @@ mod tests { 
                .unwrap();

                .unwrap();

            assert_eq!(value.meta.target, None);

            assert_eq!(value.meta.target, None);

        }

        }

        // single author

        let value = r#"

            message = "Fix typos in module documentation for generated crates"

            references = ["smithy-rs#920"]

            meta = { "breaking" = false, "tada" = false, "bug" = false }

            author = "rcoh"

        "#;

        {

            let value: HandAuthoredEntry = toml::from_str(value)

                .context("String should have parsed with multiple authors")

                .unwrap();

            assert_eq!(value.authors, Authors(vec!["rcoh".to_string()]));

        }

        // multiple authors

        let value = r#"

            message = "Fix typos in module documentation for generated crates"

            references = ["smithy-rs#920"]

            meta = { "breaking" = false, "tada" = false, "bug" = false }

            authors = ["rcoh", "crisidev"]

        "#;

        {

            let value: HandAuthoredEntry = toml::from_str(value)

                .context("String should have parsed with multiple authors")

                .unwrap();

            assert_eq!(

                value.authors,

                Authors(vec!["rcoh".to_string(), "crisidev".to_string()])

            );

        }

    }

    }

}
 
}