Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'released as part of' notes to issues when releasing #1654

Merged
merged 3 commits into from Oct 13, 2019

Conversation

blairconrad
Copy link
Member

Fixes #1652.

@blairconrad
Copy link
Member Author

I went for option 3—adding a note during pre-release and trying not to repeat it later. Parsing the numbers from the previous releases was easier than my initial idea of scraping the issues themselves.

@blairconrad
Copy link
Member Author

If you want, you can see where I tested on https://github.com/blairconrad/FakeItEasy/releases/tag/5.5.0 and https://github.com/blairconrad/FakeItEasy/releases/tag/5.5.0-beta.1
I actually used the releases twice, so you can see notes for 5.4 and 5.5 in there. I included a PR to make sure that would get updated, and one of the issues was "previously fixed" in a beta, so it didn't get a note in the new releases.

Copy link
Member

@thomaslevesque thomaslevesque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, @blairconrad! Simple and to the point.
I just have one minor remark, and a suggestion.

}

string baseName = release.Name.Split('-')[0];
return releases.Where(r => r.Prerelease && r.Name.StartsWith(baseName));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this filter is sufficient. If the release is 5.1.1, it will match 5.1.10-beta.1.

Of course, under normal circumstances 5.1.10-xxx wouldn't exist... But I think it wouldn't hurt to include the dash in basename, i.e. match only issues starting with 5.1.1-.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought of this last night. And was too lazy. In addition to thinking that 5.1.10-xxx wouldn't exist, even if it did, it just means we're remove issues attributed to it from the thanking mechanism for 5.1.1, which... is fine?

But I'm happy to change.

@@ -63,9 +64,29 @@ public static async Task Main(string[] args)
await UploadPackageToNuGetAsync(file, nugetServerUrl, nugetApiKey);
}

var issueNumbersInCurrentRelease = ReleaseHelpers.GetIssueNumbersReferencedFromReleases(new[] { release });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe consider using static ReleaseHelpers? If you prefer to keep it explicit, it's fine by me, but I just wanted to mention the possibility.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@blairconrad
Copy link
Member Author

Thanks for the comments, @thomaslevesque. I pushed fixups. I figured I'd wait for the rebase until I'm squashing anyhow…

string baseName = BaseName(release);
return releases.Where(r => r.Prerelease && BaseName(r) == baseName);

string BaseName(Release release) => release.Name.Split('-')[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 👍

thomaslevesque
thomaslevesque previously approved these changes Oct 13, 2019
Copy link
Member

@thomaslevesque thomaslevesque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @blairconrad. Squash it up!

@blairconrad
Copy link
Member Author

Thanks, @thomaslevesque. Squashed.

@thomaslevesque thomaslevesque merged commit b5b6ba5 into FakeItEasy:master Oct 13, 2019
@blairconrad blairconrad deleted the auto-issue branch October 13, 2019 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically add "this change has been released" note to issues
2 participants