Skip to content

Commit 87f1b1b

Browse files
CopilotMalcolmnixon
andcommitted
Address PR feedback: simplify code, use var, rename variables, fix commit range exclusion
Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>
1 parent e4dd1b9 commit 87f1b1b

File tree

1 file changed

+46
-62
lines changed

1 file changed

+46
-62
lines changed

src/DemaConsulting.BuildMark/RepoConnectors/GitHubRepoConnector.cs

Lines changed: 46 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ namespace DemaConsulting.BuildMark;
2525
/// <summary>
2626
/// GitHub repository connector implementation using Octokit.Net.
2727
/// </summary>
28-
public partial class GitHubRepoConnector : RepoConnectorBase
28+
public class GitHubRepoConnector : RepoConnectorBase
2929
{
3030
private static readonly Dictionary<string, string> LabelTypeMap = new()
3131
{
@@ -44,7 +44,7 @@ public partial class GitHubRepoConnector : RepoConnectorBase
4444
/// <param name="version">Optional target version. If not provided, uses the most recent tag if it matches current commit.</param>
4545
/// <returns>BuildInformation record with all collected data.</returns>
4646
/// <exception cref="InvalidOperationException">Thrown if version cannot be determined.</exception>
47-
public override async Task<BuildInformation> GetBuildInformationAsync(DemaConsulting.BuildMark.Version? version = null)
47+
public override async Task<BuildInformation> GetBuildInformationAsync(Version? version = null)
4848
{
4949
// Get repository metadata using git commands
5050
var repoUrl = await RunCommandAsync("git", "remote get-url origin");
@@ -80,7 +80,9 @@ public override async Task<BuildInformation> GetBuildInformationAsync(DemaConsul
8080

8181
// Build a mapping from commit SHA to pull request.
8282
// This is used to associate commits with their pull requests for change tracking.
83-
var commitHashToPr = BuildCommitToPrMap(pullRequests);
83+
var commitHashToPr = pullRequests
84+
.Where(p => p.Merged && p.MergeCommitSha != null)
85+
.ToDictionary(p => p.MergeCommitSha!, p => p);
8486

8587
// Build a set of commit SHAs in the current branch.
8688
// This is used for efficient filtering of branch-related tags.
@@ -109,68 +111,66 @@ public override async Task<BuildInformation> GetBuildInformationAsync(DemaConsul
109111
// Parse release tags into Version objects, maintaining release order (newest to oldest).
110112
// This is used to determine version history and find previous releases.
111113
var releaseVersions = branchReleases
112-
.Select(r => DemaConsulting.BuildMark.Version.TryCreate(r.TagName!))
114+
.Select(r => Version.TryCreate(r.TagName!))
113115
.Where(v => v != null)
114-
.Cast<DemaConsulting.BuildMark.Version>()
116+
.Cast<Version>()
115117
.ToList();
116118

117119
// Determine the target version and hash for build information
118-
DemaConsulting.BuildMark.Version toTagInfo;
119-
string toHash = currentCommitHash.Trim();
120-
if (version != null)
120+
var toVersion = version;
121+
var toHash = currentCommitHash.Trim();
122+
if (toVersion == null)
121123
{
122-
// Use explicitly specified version as target
123-
toTagInfo = version;
124-
}
125-
else if (releaseVersions.Count > 0)
126-
{
127-
// Use the most recent release (first in list since releases are newest to oldest)
128-
var latestRelease = branchReleases[0];
129-
var latestReleaseVersion = releaseVersions[0];
130-
var latestTagCommit = tagsByName[latestRelease.TagName!];
131-
132-
if (latestTagCommit.Commit.Sha == toHash)
124+
if (releaseVersions.Count > 0)
133125
{
134-
// Current commit matches latest release tag, use it as target
135-
toTagInfo = latestReleaseVersion;
126+
// Use the most recent release (first in list since releases are newest to oldest)
127+
var latestRelease = branchReleases[0];
128+
var latestReleaseVersion = releaseVersions[0];
129+
var latestTagCommit = tagsByName[latestRelease.TagName!];
130+
131+
if (latestTagCommit.Commit.Sha == toHash)
132+
{
133+
// Current commit matches latest release tag, use it as target
134+
toVersion = latestReleaseVersion;
135+
}
136+
else
137+
{
138+
// Current commit doesn't match any release tag, cannot determine version
139+
throw new InvalidOperationException(
140+
"Target version not specified and current commit does not match any release tag. " +
141+
"Please provide a version parameter.");
142+
}
136143
}
137144
else
138145
{
139-
// Current commit doesn't match any release tag, cannot determine version
146+
// No releases in repository and no version provided
140147
throw new InvalidOperationException(
141-
"Target version not specified and current commit does not match any release tag. " +
148+
"No releases found in repository and no version specified. " +
142149
"Please provide a version parameter.");
143150
}
144151
}
145-
else
146-
{
147-
// No releases in repository and no version provided
148-
throw new InvalidOperationException(
149-
"No releases found in repository and no version specified. " +
150-
"Please provide a version parameter.");
151-
}
152152

153153
// Determine the starting release for comparing changes
154-
DemaConsulting.BuildMark.Version? fromTagInfo = null;
154+
Version? fromVersion = null;
155155
string? fromHash = null;
156156

157157
if (releaseVersions.Count > 0)
158158
{
159159
// Find the position of target version in release history
160-
var toIndex = FindVersionIndex(releaseVersions, toTagInfo.FullVersion);
160+
var toIndex = FindVersionIndex(releaseVersions, toVersion.FullVersion);
161161

162-
if (toTagInfo.IsPreRelease)
162+
if (toVersion.IsPreRelease)
163163
{
164164
// Pre-release versions use the immediately previous (older) release as baseline
165165
if (toIndex >= 0 && toIndex < releaseVersions.Count - 1)
166166
{
167167
// Target version exists in history, use next older release (higher index)
168-
fromTagInfo = releaseVersions[toIndex + 1];
168+
fromVersion = releaseVersions[toIndex + 1];
169169
}
170170
else if (toIndex == -1 && releaseVersions.Count > 0)
171171
{
172172
// Target version not in history, use most recent (first) release as baseline
173-
fromTagInfo = releaseVersions[0];
173+
fromVersion = releaseVersions[0];
174174
}
175175
// If toIndex is last in list, this is the oldest release, no baseline
176176
}
@@ -202,16 +202,16 @@ public override async Task<BuildInformation> GetBuildInformationAsync(DemaConsul
202202
{
203203
if (!releaseVersions[i].IsPreRelease)
204204
{
205-
fromTagInfo = releaseVersions[i];
205+
fromVersion = releaseVersions[i];
206206
break;
207207
}
208208
}
209209
}
210210
}
211211

212212
// Get commit hash for baseline version if one was found
213-
if (fromTagInfo != null &&
214-
tagToRelease.TryGetValue(fromTagInfo.Tag, out var fromRelease) &&
213+
if (fromVersion != null &&
214+
tagToRelease.TryGetValue(fromVersion.Tag, out var fromRelease) &&
215215
tagsByName.TryGetValue(fromRelease.TagName!, out var fromTagCommit))
216216
{
217217
fromHash = fromTagCommit.Commit.Sha;
@@ -299,8 +299,8 @@ public override async Task<BuildInformation> GetBuildInformationAsync(DemaConsul
299299

300300
// Create and return build information with all collected data
301301
return new BuildInformation(
302-
fromTagInfo,
303-
toTagInfo,
302+
fromVersion,
303+
toVersion,
304304
fromHash,
305305
toHash,
306306
nonBugChanges,
@@ -323,29 +323,12 @@ private static async Task<IReadOnlyList<GitHubCommit>> GetAllCommitsAsync(GitHub
323323
}
324324

325325
/// <summary>
326-
/// Builds a map from commit hash (merge SHA) to pull request.
327-
/// </summary>
328-
/// <param name="pullRequests">List of pull requests.</param>
329-
/// <returns>Dictionary mapping commit hash to pull request.</returns>
330-
private static Dictionary<string, PullRequest> BuildCommitToPrMap(IReadOnlyList<PullRequest> pullRequests)
331-
{
332-
var map = new Dictionary<string, PullRequest>();
333-
334-
foreach (var pr in pullRequests.Where(p => p.Merged && p.MergeCommitSha != null))
335-
{
336-
map[pr.MergeCommitSha] = pr;
337-
}
338-
339-
return map;
340-
}
341-
342-
/// <summary>
343-
/// Gets commits in the range from fromHash to toHash.
326+
/// Gets commits in the range from fromHash (exclusive) to toHash (inclusive).
344327
/// </summary>
345328
/// <param name="commits">All commits.</param>
346-
/// <param name="fromHash">Starting commit hash (null for start of history).</param>
347-
/// <param name="toHash">Ending commit hash.</param>
348-
/// <returns>List of commits in range.</returns>
329+
/// <param name="fromHash">Starting commit hash (exclusive - not included in results; null for start of history).</param>
330+
/// <param name="toHash">Ending commit hash (inclusive - included in results).</param>
331+
/// <returns>List of commits in range, excluding fromHash but including toHash.</returns>
349332
private static List<GitHubCommit> GetCommitsInRange(IReadOnlyList<GitHubCommit> commits, string? fromHash, string toHash)
350333
{
351334
var result = new List<GitHubCommit>();
@@ -359,8 +342,9 @@ private static List<GitHubCommit> GetCommitsInRange(IReadOnlyList<GitHubCommit>
359342
foundTo = true;
360343
}
361344

362-
if (foundTo)
345+
if (foundTo && commit.Sha != fromHash)
363346
{
347+
// Skip the fromHash commit itself - we want changes AFTER the last release
364348
result.Add(commit);
365349
}
366350

0 commit comments

Comments
 (0)