From a78bfe1abb90318d0a5702fd0c3963faca270a2c Mon Sep 17 00:00:00 2001 From: Antoine van der Lee <4329185+AvdLee@users.noreply.github.com> Date: Tue, 13 Jun 2023 14:57:50 +0200 Subject: [PATCH 1/4] Make sure GH release notes are used correctly with previous tag --- Package.resolved | 2 +- .../Changelog/ChangelogProducer.swift | 32 +++++++++++++- .../Commands/ReleaseCommand.swift | 2 +- .../GitHub/GitHubReleaseNotesGenerator.swift | 44 +++++++++++++++++++ Sources/GitBuddyCore/Models/Changelog.swift | 8 ++++ .../Release/ReleaseProducer.swift | 11 +++-- .../Release/ReleaseProducerTests.swift | 22 ++++++++++ Tests/GitBuddyTests/TestHelpers/Mocks.swift | 12 +++++ .../TestHelpers/ReleaseNotesJSON.swift | 17 +++++++ 9 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift create mode 100644 Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift diff --git a/Package.resolved b/Package.resolved index b6ac9e4..dce8a43 100644 --- a/Package.resolved +++ b/Package.resolved @@ -15,7 +15,7 @@ "repositoryURL": "https://github.com/WeTransfer/octokit.swift", "state": { "branch": "main", - "revision": "3d0fea9587af530cb13ef5801a3cb90186fce43e", + "revision": "d3706890a06d2f9afc1de4665191167058438153", "version": null } }, diff --git a/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift b/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift index a3d7833..1b510dc 100644 --- a/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift +++ b/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift @@ -38,8 +38,20 @@ final class ChangelogProducer: URLSessionInjectable { let from: Date let to: Date let project: GITProject + let useGitHubReleaseNotes: Bool + let tagName: String? + let targetCommitish: String? + let previousTagName: String? - init(since: Since = .latestTag, to: Date = Date(), baseBranch: Branch?) throws { + init( + since: Since = .latestTag, + to: Date = Date(), + baseBranch: Branch?, + useGitHubReleaseNotes: Bool = false, + tagName: String? = nil, + targetCommitish: String? = nil, + previousTagName: String? = nil + ) throws { try Octokit.authenticate() self.to = to @@ -57,9 +69,27 @@ final class ChangelogProducer: URLSessionInjectable { Log.debug("Getting all changes between \(self.from) and \(self.to)") self.baseBranch = baseBranch ?? "master" project = GITProject.current() + self.useGitHubReleaseNotes = useGitHubReleaseNotes + self.tagName = tagName + self.targetCommitish = targetCommitish + self.previousTagName = previousTagName } @discardableResult public func run(isSectioned: Bool) throws -> Changelog { + if useGitHubReleaseNotes, let tagName, let targetCommitish, let previousTagName { + return try GitHubReleaseNotesGenerator( + octoKit: octoKit, + project: project, + tagName: tagName, + targetCommitish: targetCommitish, + previousTagName: previousTagName + ).generate(using: urlSession) + } else { + return try generateChangelogUsingPRsAndIssues(isSectioned: isSectioned) + } + } + + private func generateChangelogUsingPRsAndIssues(isSectioned: Bool) throws -> Changelog { let pullRequestsFetcher = PullRequestFetcher(octoKit: octoKit, baseBranch: baseBranch, project: project) let pullRequests = try pullRequestsFetcher.fetchAllBetween(from, and: to, using: urlSession) diff --git a/Sources/GitBuddyCore/Commands/ReleaseCommand.swift b/Sources/GitBuddyCore/Commands/ReleaseCommand.swift index dfe545e..794a838 100644 --- a/Sources/GitBuddyCore/Commands/ReleaseCommand.swift +++ b/Sources/GitBuddyCore/Commands/ReleaseCommand.swift @@ -72,7 +72,7 @@ struct ReleaseCommand: ParsableCommand { @Flag(name: .customLong("sections"), help: "Whether the changelog should be split into sections. Defaults to false.") private var isSectioned: Bool = false - @Flag(name: .customLong("useGitHubReleaseNotes"), help: "If `true`, release notes will be generated by GitHub. Defaults to false, which will lead to a changelog based on PR and issue titles matching the tag changes.") + @Flag(name: .customLong("use-github-release-notes"), help: "If `true`, release notes will be generated by GitHub. Defaults to false, which will lead to a changelog based on PR and issue titles matching the tag changes.") private var useGitHubReleaseNotes: Bool = false @Flag(name: .customLong("json"), help: "Whether the release output should be in JSON, containing more details. Defaults to false.") diff --git a/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift b/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift new file mode 100644 index 0000000..ee87399 --- /dev/null +++ b/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift @@ -0,0 +1,44 @@ +// +// GitHubReleaseNotesGenerator.swift +// +// +// Created by Antoine van der Lee on 13/06/2023. +// + +import Foundation +import OctoKit + +struct GitHubReleaseNotesGenerator { + let octoKit: Octokit + let project: GITProject + let tagName: String + let targetCommitish: String + let previousTagName: String + + func generate(using session: URLSession = URLSession.shared) throws -> ReleaseNotes { + let group = DispatchGroup() + group.enter() + + var result: Result! + + octoKit.generateReleaseNotes( + session, + owner: project.organisation, + repository: project.repository, + tagName: tagName, + targetCommitish: targetCommitish, + previousTagName: previousTagName) { response in + switch response { + case .success(let releaseNotes): + result = .success(releaseNotes) + case .failure(let error): + result = .failure(OctoKitError(error: error)) + } + group.leave() + } + + group.wait() + + return try result.get() + } +} diff --git a/Sources/GitBuddyCore/Models/Changelog.swift b/Sources/GitBuddyCore/Models/Changelog.swift index e82ab03..3d68939 100644 --- a/Sources/GitBuddyCore/Models/Changelog.swift +++ b/Sources/GitBuddyCore/Models/Changelog.swift @@ -20,6 +20,14 @@ protocol Changelog: CustomStringConvertible { var itemIdentifiers: [PullRequestID: [IssueID]] { get } } +extension ReleaseNotes: Changelog { + var itemIdentifiers: [PullRequestID : [IssueID]] { + [:] + } + + public var description: String { body } +} + /// Represents a changelog with a single section of changelog items. struct SingleSectionChangelog: Changelog { let description: String diff --git a/Sources/GitBuddyCore/Release/ReleaseProducer.swift b/Sources/GitBuddyCore/Release/ReleaseProducer.swift index 5ab1e1c..b63a07f 100644 --- a/Sources/GitBuddyCore/Release/ReleaseProducer.swift +++ b/Sources/GitBuddyCore/Release/ReleaseProducer.swift @@ -69,17 +69,21 @@ final class ReleaseProducer: URLSessionInjectable, ShellInjectable { /// We're adding 60 seconds to make sure the tag commit itself is included in the changelog as well. let adjustedChangelogToDate = changelogToDate.addingTimeInterval(60) + let tagName = try tagName ?? Tag.latest().name let changelogSinceTag = lastReleaseTag ?? Self.shell.execute(.previousTag) let changelogProducer = try ChangelogProducer( since: .tag(tag: changelogSinceTag), to: adjustedChangelogToDate, - baseBranch: baseBranch + baseBranch: baseBranch, + useGitHubReleaseNotes: useGitHubReleaseNotes, + tagName: tagName, + targetCommitish: targetCommitish, + previousTagName: changelogSinceTag ) let changelog = try changelogProducer.run(isSectioned: isSectioned) Log.debug("\(changelog)\n") - let tagName = try tagName ?? Tag.latest().name try updateChangelogFile(adding: changelog.description, for: tagName) let repositoryName = Self.shell.execute(.repositoryName) @@ -206,7 +210,8 @@ final class ReleaseProducer: URLSessionInjectable, ShellInjectable { body: body, prerelease: isPrerelease, draft: false, - generateReleaseNotes: useGitHubReleaseNotes + /// Since GitHub's API does not support setting `previous_tag_name`, we manually call the API to generate automated GH changelogs. + generateReleaseNotes: false ) { response in switch response { case .success(let release): diff --git a/Tests/GitBuddyTests/Release/ReleaseProducerTests.swift b/Tests/GitBuddyTests/Release/ReleaseProducerTests.swift index 6b03149..55e38e2 100644 --- a/Tests/GitBuddyTests/Release/ReleaseProducerTests.swift +++ b/Tests/GitBuddyTests/Release/ReleaseProducerTests.swift @@ -78,6 +78,28 @@ final class ReleaseProducerTests: XCTestCase { wait(for: [mockExpectation], timeout: 0.3) } + /// It should set the parameters correctly. + func testPostBodyArgumentsGitHubReleaseNotes() throws { + let mockExpectation = expectation(description: "Mocks should be called") + Mocker.mockReleaseNotes() + var mock = Mocker.mockRelease() + mock.onRequest = { _, parameters in + guard let parameters = try? XCTUnwrap(parameters) else { return } + XCTAssertEqual(parameters["prerelease"] as? Bool, false) + XCTAssertEqual(parameters["draft"] as? Bool, false) + XCTAssertEqual(parameters["tag_name"] as? String, "1.0.1") + XCTAssertEqual(parameters["name"] as? String, "1.0.1") + XCTAssertEqual(parameters["body"] as? String, """ + ##Changes in Release v1.0.0 ... ##Contributors @monalisa + """) + mockExpectation.fulfill() + } + mock.register() + + try executeCommand("gitbuddy release -s --use-github-release-notes -t develop") + wait(for: [mockExpectation], timeout: 0.3) + } + /// It should update the changelog file if the argument is set. func testChangelogUpdating() throws { let existingChangelog = """ diff --git a/Tests/GitBuddyTests/TestHelpers/Mocks.swift b/Tests/GitBuddyTests/TestHelpers/Mocks.swift index fb13dc7..97eb2b7 100644 --- a/Tests/GitBuddyTests/TestHelpers/Mocks.swift +++ b/Tests/GitBuddyTests/TestHelpers/Mocks.swift @@ -124,6 +124,18 @@ extension Mocker { return mock } + @discardableResult static func mockReleaseNotes() -> Mock { + let releaseNotesJSONData = ReleaseNotesJSON.data(using: .utf8)! + let mock = Mock( + url: URL(string: "https://api.github.com/repos/WeTransfer/Diagnostics/releases/generate-notes")!, + dataType: .json, + statusCode: 200, + data: [.post: releaseNotesJSONData] + ) + mock.register() + return mock + } + @discardableResult static func mockListReleases() -> Mock { let releaseJSONData = ListReleasesJSON.data(using: .utf8)! let mock = Mock( diff --git a/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift b/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift new file mode 100644 index 0000000..1bb8611 --- /dev/null +++ b/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift @@ -0,0 +1,17 @@ +// +// ReleaseNotesJSON.swift +// +// +// Created by Antoine van der Lee on 13/06/2023. +// + +import Foundation + +// swiftlint:disable line_length +let ReleaseNotesJSON = """ + +{ + "name": "Release v1.0.0 is now available!", + "body": "##Changes in Release v1.0.0 ... ##Contributors @monalisa" +} +""" From 83e673091a17de8bc33674876ce219876f31ce86 Mon Sep 17 00:00:00 2001 From: Antoine van der Lee <4329185+AvdLee@users.noreply.github.com> Date: Wed, 14 Jun 2023 11:59:43 +0200 Subject: [PATCH 2/4] Add header copyright --- Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift | 1 + Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift | 1 + 2 files changed, 2 insertions(+) diff --git a/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift b/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift index ee87399..4cee823 100644 --- a/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift +++ b/Sources/GitBuddyCore/GitHub/GitHubReleaseNotesGenerator.swift @@ -3,6 +3,7 @@ // // // Created by Antoine van der Lee on 13/06/2023. +// Copyright © 2023 WeTransfer. All rights reserved. // import Foundation diff --git a/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift b/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift index 1bb8611..030e392 100644 --- a/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift +++ b/Tests/GitBuddyTests/TestHelpers/ReleaseNotesJSON.swift @@ -3,6 +3,7 @@ // // // Created by Antoine van der Lee on 13/06/2023. +// Copyright © 2023 WeTransfer. All rights reserved. // import Foundation From e8967396437b636bdd0008d0f6617c8431e509db Mon Sep 17 00:00:00 2001 From: Antoine van der Lee <4329185+AvdLee@users.noreply.github.com> Date: Wed, 14 Jun 2023 12:04:34 +0200 Subject: [PATCH 3/4] Add parameter documentation for Changelog and Release producer --- .../Changelog/ChangelogProducer.swift | 19 ++++++++++ .../Release/ReleaseProducer.swift | 35 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift b/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift index 1b510dc..4054ea2 100644 --- a/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift +++ b/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift @@ -33,14 +33,33 @@ final class ChangelogProducer: URLSessionInjectable { } private lazy var octoKit: Octokit = .init() + + /// The base branch to compare with. Defaults to master. let baseBranch: Branch + + /// "The tag to use as a base. Defaults to the latest tag. let since: Since + let from: Date let to: Date + + /// The GIT Project to create a changelog for. let project: GITProject + + /// If `true`, release notes will be generated by GitHub. + /// Defaults to false, which will lead to a changelog based on PR and issue titles matching the tag changes. let useGitHubReleaseNotes: Bool + + /// The name of the tag to use as base for the changelog comparison. let tagName: String? + + /// Specifies the commitish value that will be the target for the release's tag. + /// Required if the supplied tagName does not reference an existing tag. Ignored if the tagName already exists. + /// While we're talking about creating tags, the changelog producer will only use these values for GitHub release + /// rotes generation. let targetCommitish: String? + + /// The previous tag to compare against. Will only be used for GitHub release notes generation. let previousTagName: String? init( diff --git a/Sources/GitBuddyCore/Release/ReleaseProducer.swift b/Sources/GitBuddyCore/Release/ReleaseProducer.swift index b63a07f..009ff1b 100644 --- a/Sources/GitBuddyCore/Release/ReleaseProducer.swift +++ b/Sources/GitBuddyCore/Release/ReleaseProducer.swift @@ -23,15 +23,50 @@ final class ReleaseProducer: URLSessionInjectable, ShellInjectable { } private lazy var octoKit: Octokit = .init() + + /// The path to the Changelog to update it with the latest changes. let changelogURL: Foundation.URL? + + /// Disable commenting on issues and PRs about the new release. let skipComments: Bool + + /// Create the release as a pre-release. let isPrerelease: Bool + + /* + Specifies the commitish value that determines where the Git tag is created from. Can be any branch or commit SHA. + Unused if the Git tag already exists. + + Default: the repository's default branch (usually main). + */ let targetCommitish: String? + + /* + The name of the tag. If set, `changelogToTag` is required too. + + Default: takes the last created tag to publish as a GitHub release. + */ let tagName: String? + + /// The title of the release. Default: uses the tag name. let releaseTitle: String? + + /// The last release tag to use as a base for the changelog creation. Default: previous tag. let lastReleaseTag: String? + + /* + If set, the date of this tag will be used as the limit for the changelog creation. + This variable should be passed when `tagName` is set. + + Default: latest tag. + */ let changelogToTag: String? + + /// The base branch to compare with for generating the changelog. Defaults to master. let baseBranch: String + + /// If `true`, release notes will be generated by GitHub. + /// Defaults to false, which will lead to a changelog based on PR and issue titles matching the tag changes. let useGitHubReleaseNotes: Bool init( From 8d820fa4bf67ad2e8f3fb661c6386f86b2c8c353 Mon Sep 17 00:00:00 2001 From: Antoine van der Lee <4329185+AvdLee@users.noreply.github.com> Date: Wed, 14 Jun 2023 12:08:56 +0200 Subject: [PATCH 4/4] Remove force unwraps for data --- .../Changelog/ChangelogItemsFactoryTests.swift | 6 +++--- .../GitBuddyTests/Models/ChangelogItemTests.swift | 4 ++-- Tests/GitBuddyTests/TestHelpers/Mocks.swift | 14 +++++++------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Tests/GitBuddyTests/Changelog/ChangelogItemsFactoryTests.swift b/Tests/GitBuddyTests/Changelog/ChangelogItemsFactoryTests.swift index 59d0bc6..6c77d12 100644 --- a/Tests/GitBuddyTests/Changelog/ChangelogItemsFactoryTests.swift +++ b/Tests/GitBuddyTests/Changelog/ChangelogItemsFactoryTests.swift @@ -31,7 +31,7 @@ final class ChangelogItemsFactoryTests: XCTestCase { /// It should return the pull request only if no referencing issues are found. func testCreatingItems() { - let pullRequest = PullRequestsJSON.data(using: .utf8)!.mapJSON(to: [PullRequest].self).first! + let pullRequest = Data(PullRequestsJSON.utf8).mapJSON(to: [PullRequest].self).first! let factory = ChangelogItemsFactory(octoKit: octoKit, pullRequests: [pullRequest], project: project) let items = factory.items(using: urlSession) XCTAssertEqual(items.count, 1) @@ -41,8 +41,8 @@ final class ChangelogItemsFactoryTests: XCTestCase { /// It should return the referencing issue with the pull request. func testReferencingIssue() { - let pullRequest = PullRequestsJSON.data(using: .utf8)!.mapJSON(to: [PullRequest].self).last! - let issue = IssueJSON.data(using: .utf8)!.mapJSON(to: Issue.self) + let pullRequest = Data(PullRequestsJSON.utf8).mapJSON(to: [PullRequest].self).last! + let issue = Data(IssueJSON.utf8).mapJSON(to: Issue.self) let factory = ChangelogItemsFactory(octoKit: octoKit, pullRequests: [pullRequest], project: project) Mocker.mockForIssueNumber(39) let items = factory.items(using: urlSession) diff --git a/Tests/GitBuddyTests/Models/ChangelogItemTests.swift b/Tests/GitBuddyTests/Models/ChangelogItemTests.swift index fa47ae0..644569e 100644 --- a/Tests/GitBuddyTests/Models/ChangelogItemTests.swift +++ b/Tests/GitBuddyTests/Models/ChangelogItemTests.swift @@ -38,7 +38,7 @@ final class ChangelogItemTests: XCTestCase { /// It should show the user if possible. func testUser() { - let input = PullRequestsJSON.data(using: .utf8)!.mapJSON(to: [PullRequest].self).first! + let input = Data(PullRequestsJSON.utf8).mapJSON(to: [PullRequest].self).first! input.htmlURL = nil let item = ChangelogItem(input: input, closedBy: input) XCTAssertEqual(item.title, "Add charset utf-8 to html head via [@AvdLee](https://github.com/AvdLee)") @@ -46,7 +46,7 @@ final class ChangelogItemTests: XCTestCase { /// It should fallback to the assignee if the user is nil for Pull Requests. func testAssigneeFallback() { - let input = PullRequestsJSON.data(using: .utf8)!.mapJSON(to: [PullRequest].self).first! + let input = Data(PullRequestsJSON.utf8).mapJSON(to: [PullRequest].self).first! input.user = nil input.htmlURL = nil let item = ChangelogItem(input: input, closedBy: input) diff --git a/Tests/GitBuddyTests/TestHelpers/Mocks.swift b/Tests/GitBuddyTests/TestHelpers/Mocks.swift index 97eb2b7..5efe0c3 100644 --- a/Tests/GitBuddyTests/TestHelpers/Mocks.swift +++ b/Tests/GitBuddyTests/TestHelpers/Mocks.swift @@ -80,7 +80,7 @@ extension Mocker { URLQueryItem(name: "state", value: "closed") ] - let pullRequestJSONData = PullRequestsJSON.data(using: .utf8)! + let pullRequestJSONData = Data(PullRequestsJSON.utf8) let mock = Mock(url: urlComponents.url!, dataType: .json, statusCode: 200, data: [.get: pullRequestJSONData]) mock.register() } @@ -101,19 +101,19 @@ extension Mocker { URLQueryItem(name: "state", value: "closed") ] - let data = IssuesJSON.data(using: .utf8)! + let data = Data(IssuesJSON.utf8) let mock = Mock(url: urlComponents.url!, dataType: .json, statusCode: 200, data: [.get: data]) mock.register() } static func mockForIssueNumber(_ issueNumber: Int) { let urlComponents = URLComponents(string: "https://api.github.com/repos/WeTransfer/Diagnostics/issues/\(issueNumber)")! - let issueJSONData = IssueJSON.data(using: .utf8)! + let issueJSONData = Data(IssueJSON.utf8) Mock(url: urlComponents.url!, dataType: .json, statusCode: 200, data: [.get: issueJSONData]).register() } @discardableResult static func mockRelease() -> Mock { - let releaseJSONData = ReleaseJSON.data(using: .utf8)! + let releaseJSONData = Data(ReleaseJSON.utf8) let mock = Mock( url: URL(string: "https://api.github.com/repos/WeTransfer/Diagnostics/releases")!, dataType: .json, @@ -125,7 +125,7 @@ extension Mocker { } @discardableResult static func mockReleaseNotes() -> Mock { - let releaseNotesJSONData = ReleaseNotesJSON.data(using: .utf8)! + let releaseNotesJSONData = Data(ReleaseNotesJSON.utf8) let mock = Mock( url: URL(string: "https://api.github.com/repos/WeTransfer/Diagnostics/releases/generate-notes")!, dataType: .json, @@ -137,7 +137,7 @@ extension Mocker { } @discardableResult static func mockListReleases() -> Mock { - let releaseJSONData = ListReleasesJSON.data(using: .utf8)! + let releaseJSONData = Data(ListReleasesJSON.utf8) let mock = Mock( url: URL(string: "https://api.github.com/repos/WeTransfer/Diagnostics/releases?per_page=100")!, dataType: .json, @@ -172,7 +172,7 @@ extension Mocker { static func mockForCommentingOn(issueNumber: Int) -> Mock { let urlComponents = URLComponents(string: "https://api.github.com/repos/WeTransfer/Diagnostics/issues/\(issueNumber)/comments")! - let commentJSONData = CommentJSON.data(using: .utf8)! + let commentJSONData = Data(CommentJSON.utf8) return Mock(url: urlComponents.url!, dataType: .json, statusCode: 201, data: [.post: commentJSONData]) } }