Unverified Commit a08c9bf7 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Don't run message length validation on `SDK_CHANGELOG.next.json` (#2940) 

The new changelog message length validation is running when attempting
to release the SDK, which reads historical entries from
`SDK_CHANGELOG.next.json`, and these older entries didn't have a length
validation. Thus, this validation can't run during render time.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 9f6e69fe

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

+2 −2
Original line number Diff line number Diff line
@@ -10,7 +10,7 @@ use once_cell::sync::Lazy; 
use ordinal::Ordinal;

use serde::Serialize;

use smithy_rs_tool_common::changelog::{

    Changelog, HandAuthoredEntry, Reference, SdkModelChangeKind, SdkModelEntry,

    Changelog, HandAuthoredEntry, Reference, SdkModelChangeKind, SdkModelEntry, ValidationSet,

};

use smithy_rs_tool_common::git::{find_git_repository_root, Git, GitCLI};

use smithy_rs_tool_common::versions_manifest::{CrateVersionMetadataMap, VersionsManifest};
@@ -237,7 +237,7 @@ fn load_changelogs(args: &RenderArgs) -> Result<Changelog> { 
    for source in &args.source {

        let changelog = Changelog::load_from_file(source)

            .map_err(|errs| anyhow::Error::msg(format!("failed to load {source:?}: {errs:#?}")))?;

        changelog.validate().map_err(|errs| {

        changelog.validate(ValidationSet::Render).map_err(|errs| {

            anyhow::Error::msg(format!(

                "failed to load {source:?}: {errors}",

                errors = errs.join("\n")

tools/ci-build/sdk-lints/src/changelog.rs

+8 −6
Original line number Diff line number Diff line
@@ -6,7 +6,7 @@ 
use crate::lint::LintError;

use crate::{repo_root, Check, Lint};

use anyhow::Result;

use smithy_rs_tool_common::changelog::Changelog;

use smithy_rs_tool_common::changelog::{Changelog, ValidationSet};

use std::path::{Path, PathBuf};



pub(crate) struct ChangelogNext;
@@ -33,7 +33,9 @@ impl Check for ChangelogNext { 
/// Validate that `CHANGELOG.next.toml` follows best practices

fn check_changelog_next(path: impl AsRef<Path>) -> std::result::Result<(), Vec<LintError>> {

    let parsed = Changelog::load_from_file(path).map_err(|e| vec![LintError::via_display(e)])?;

    parsed.validate().map_err(|errs| {

    parsed

        .validate(ValidationSet::Development)

        .map_err(|errs| {

            errs.into_iter()

                .map(LintError::via_display)

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

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

+22 −9
Original line number Diff line number Diff line
@@ -139,7 +139,7 @@ pub struct HandAuthoredEntry { 


impl HandAuthoredEntry {

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

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

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

        if self.author.is_empty() {

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

        }
@@ -149,7 +149,7 @@ impl HandAuthoredEntry { 
        if self.references.is_empty() {

            bail!("Changelog entry must refer to at least one pull request or issue");

        }

        if self.message.len() > 800 {

        if validation_set == ValidationSet::Development && self.message.len() > 800 {

            bail!(

                "Your changelog entry is too long. Post long-form change log entries in \

                the GitHub Discussions under the Changelog category, and link to them from \
@@ -179,6 +179,19 @@ pub struct SdkModelEntry { 
    pub message: String,

}



#[derive(Copy, Clone, Eq, PartialEq)]

pub enum ValidationSet {

    /// Validate for local development and CI

    Development,

    /// Validate for rendering.

    ///

    /// This does less validation to avoid blocking a release for things that

    /// were added to changelog validation later that could cause issues with

    /// SDK_CHANGELOG.next.json where there are historical entries that didn't

    /// have this validation applied.

    Render,

}



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

pub struct Changelog {

    #[serde(rename = "smithy-rs")]
@@ -243,9 +256,9 @@ impl Changelog { 
        serde_json::to_string_pretty(self).context("failed to serialize changelog JSON")

    }



    pub fn validate(&self) -> Result<(), Vec<String>> {

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

        let validate_aws_handauthored = |entry: &HandAuthoredEntry| -> Result<()> {

            entry.validate()?;

            entry.validate(validation_set)?;

            if entry.meta.target.is_some() {

                bail!("aws-sdk-rust changelog entry cannot have an affected target");

            }
@@ -253,7 +266,7 @@ impl Changelog { 
        };



        let validate_smithyrs_handauthored = |entry: &HandAuthoredEntry| -> Result<()> {

            entry.validate()?;

            entry.validate(validation_set)?;

            if entry.meta.target.is_none() {

                bail!("smithy-rs entry must have an affected target");

            }
@@ -278,7 +291,7 @@ impl Changelog { 


#[cfg(test)]

mod tests {

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

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

    use anyhow::Context;



    #[test]
@@ -348,7 +361,7 @@ mod tests { 
        "#;

        // three errors should be produced, missing authors x 2 and a SdkAffected is not set to default

        let changelog: Changelog = toml::from_str(buffer).expect("valid changelog");

        let res = changelog.validate();

        let res = changelog.validate(ValidationSet::Development);

        assert!(res.is_err());

        if let Err(e) = res {

            assert_eq!(e.len(), 3);
@@ -378,7 +391,7 @@ mod tests { 
        {

            // loading directly from toml::from_str won't set the default target field

            let changelog: Changelog = toml::from_str(buffer).expect("valid changelog");

            let res = changelog.validate();

            let res = changelog.validate(ValidationSet::Development);

            assert!(res.is_err());

            if let Err(e) = res {

                assert!(e.contains(&"smithy-rs entry must have an affected target".to_string()))
@@ -387,7 +400,7 @@ mod tests { 
        {

            // loading through Chanelog will result in no error

            let changelog: Changelog = Changelog::parse_str(buffer).expect("valid changelog");

            let res = changelog.validate();

            let res = changelog.validate(ValidationSet::Development);

            assert!(res.is_ok());

            if let Err(e) = res {

                panic!("some error has been produced {e:?}");