Skip to content

Commit

Permalink
handle reviewer per task
Browse files Browse the repository at this point in the history
  • Loading branch information
@goaaats
goaaats committed Oct 5, 2024
1 parent 3ed535b commit c9656a0
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 66 deletions.
23 changes: 9 additions & 14 deletions Plogon/BuildProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ public class BuildProcessor
private readonly DockerClient dockerClient;

private readonly IAmazonS3? s3Client;

private readonly string? actor;

private static readonly string[] DalamudInternalDll = new[]
{
Expand Down Expand Up @@ -174,11 +172,6 @@ public struct BuildProcessorSetup
/// S3 client to use for artifact uploads.
/// </summary>
public IAmazonS3? S3Client { get; set; }

/// <summary>
/// Actor of the GitHub action that triggered this build. Null in non-ci builds.
/// </summary>
public string? GitHubActor { get; set; }
}

/// <summary>
Expand All @@ -202,8 +195,6 @@ public BuildProcessor(BuildProcessorSetup setup)
this.dockerClient = new DockerClientConfiguration().CreateClient();

this.s3Client = setup.S3Client;

this.actor = setup.GitHubActor;
}

/// <summary>
Expand Down Expand Up @@ -878,12 +869,16 @@ private static void WriteNugetConfig(FileInfo output)
/// <param name="task">The task to build</param>
/// <param name="commit">Whether the plugin should be committed to the repo</param>
/// <param name="changelog">The plugin changelog</param>
/// <param name="reviewer">Reviewer of this task</param>
/// <param name="otherTasks">All other queued tasks</param>
/// <returns>The result of the build</returns>
/// <exception cref="Exception">Generic build system errors</exception>
/// <exception cref="PluginCommitException">Error during repo commit, all no further work should be done</exception>
public async Task<BuildResult> ProcessTask(BuildTask task, bool commit, string? changelog, ISet<BuildTask> otherTasks)
public async Task<BuildResult> ProcessTask(BuildTask task, bool commit, string? changelog, string? reviewer, ISet<BuildTask>? otherTasks)
{
if (commit && string.IsNullOrWhiteSpace(reviewer))
throw new Exception("Reviewer must be set when committing");

if (task.Type == BuildTask.TaskType.Remove)
{
if (!commit)
Expand Down Expand Up @@ -1150,10 +1145,10 @@ await this.dockerClient.Containers.RemoveContainerAsync(containerCreateResponse.
version ?? throw new Exception("Committing, but version is null"),
task.Manifest.Plugin.MinimumVersion,
changelog,
this.actor ?? throw new Exception("Committing, but actor is null"),
reviewer!,
allNeeds.Select(x => (x.Name, x.Version)));

this.CommitReviewedNeeds(allNeeds);
this.CommitReviewedNeeds(allNeeds, reviewer!);

var repoOutputDir = this.pluginRepository.GetPluginOutputDirectory(task.Channel, task.InternalName);

Expand Down Expand Up @@ -1313,15 +1308,15 @@ private BuildResult.ReviewedNeed GetNeedStatus(string key, string version, State
return new(key, existingReview.ReviewedBy, version, existingReview.Version, null, type);
}

private void CommitReviewedNeeds(IEnumerable<BuildResult.ReviewedNeed> needs)
private void CommitReviewedNeeds(IEnumerable<BuildResult.ReviewedNeed> needs, string reviewer)
{
var newNeeds = needs
.Where(need => need.ReviewedBy == null)
.Select(
need => new State.Need
{
Key = need.Name,
ReviewedBy = this.actor ?? throw new Exception("Committing, but reviewer is null"),
ReviewedBy = reviewer,
Version = need.Version,
ReviewedAt = DateTime.UtcNow,
Type = need.Type,
Expand Down
22 changes: 21 additions & 1 deletion Plogon/GitHubApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public async Task<bool> CrossOutAllOfMyComments(int issueNumber)
/// </summary>
/// <param name="prNum">The pull request number</param>
/// <returns></returns>
public async Task<string> GetPullRequestDiff(string prNum)
public async Task<string> GetPullRequestDiff(int prNum)
{
using var client = new HttpClient();
return await client.GetStringAsync($"https://github.com/{repoOwner}/{repoName}/pull/{prNum}.diff");
Expand All @@ -115,6 +115,26 @@ public async Task<string> GetIssueBody(int issueNumber)

return pr.Body;
}

/// <summary>
/// Get the username of the first approving reviewer of a PR.
/// </summary>
/// <param name="issueNumber"></param>
/// <returns></returns>
/// <exception cref="Exception"></exception>
public async Task<string> GetReviewer(int issueNumber)
{
var reviews = await this.ghClient.PullRequest.Review.GetAll(repoOwner, repoName, issueNumber);
if (reviews == null)
throw new Exception("Could not get reviews");

var firstApprovingReview = reviews.FirstOrDefault(r => r.State == PullRequestReviewState.Approved &&
PlogonSystemDefine.PacMembers.Contains(r.User.Login));
if (firstApprovingReview == null)
throw new Exception($"No approving reviews on PR {issueNumber}");

return firstApprovingReview.User.Login;
}

/// <summary>
/// Get the PR for a given number.
Expand Down
2 changes: 1 addition & 1 deletion Plogon/Plogon.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<PackageReference Include="Serilog" Version="2.11.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />
<PackageReference Include="System.CommandLine.DragonFruit" Version="0.4.0-alpha.22114.1" />
<PackageReference Include="Tomlyn" Version="0.14.3" />
<PackageReference Include="Tomlyn" Version="0.17.0" />
</ItemGroup>

<ItemGroup>
Expand Down
97 changes: 54 additions & 43 deletions Plogon/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,16 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,
var repoParts = Environment.GetEnvironmentVariable("GITHUB_REPOSITORY")?.Split("/");
var repoOwner = repoParts?[0];
var repoName = repoParts?[1];
var prNumber = Environment.GetEnvironmentVariable("GITHUB_PR_NUM");

if (mode == ModeOfOperation.PullRequest && string.IsNullOrEmpty(prNumber))
throw new Exception("PR number not set");

var prNumberStr = Environment.GetEnvironmentVariable("GITHUB_PR_NUM");
int? prNumber = mode switch
{
ModeOfOperation.PullRequest when string.IsNullOrEmpty(prNumberStr) => throw new Exception(
"PR number not set"),
ModeOfOperation.PullRequest => int.Parse(prNumberStr),
_ => null
};

GitHubApi? gitHubApi = null;
if (ci)
{
Expand Down Expand Up @@ -134,7 +139,7 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,
string? prDiff = null;
if (gitHubApi is not null && repoName is not null && prNumber is not null)
{
prDiff = await gitHubApi.GetPullRequestDiff(prNumber);
prDiff = await gitHubApi.GetPullRequestDiff(prNumber.Value);
}
else if (mode == ModeOfOperation.PullRequest)
{
Expand All @@ -155,7 +160,6 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,
AllowNonDefaultImages = mode != ModeOfOperation.Continuous, // HACK, fix it
CutoffDate = null,
S3Client = s3Client,
GitHubActor = githubActor,
};

// HACK, we don't know the API level a plugin is for before building it...
Expand All @@ -169,6 +173,7 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,

var buildProcessor = new BuildProcessor(setup);
var tasks = buildProcessor.GetBuildTasks(mode == ModeOfOperation.Continuous);
var taskToPrNumber = new Dictionary<BuildTask, int>();

GitHubOutputBuilder.StartGroup("List all tasks");

Expand Down Expand Up @@ -252,7 +257,7 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,
GitHubOutputBuilder.StartGroup($"Remove {task.InternalName}");
Log.Information("Remove: {Name} - {Channel}", task.InternalName, task.Channel);

var removeStatus = await buildProcessor.ProcessTask(task, true, null, tasks);
var removeStatus = await buildProcessor.ProcessTask(task, true, null, null, tasks);
allResults.Add(removeStatus);

if (removeStatus.Success)
Expand Down Expand Up @@ -292,14 +297,27 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,

numTried++;

string? reviewer = null;
var changelog = task.Manifest.Plugin.Changelog;
if (string.IsNullOrEmpty(changelog) && repoName != null && prNumber != null &&
gitHubApi != null && mode == ModeOfOperation.Commit)
int? committingPrNum = null;

if (mode == ModeOfOperation.Commit)
{
changelog = await gitHubApi.GetIssueBody(int.Parse(prNumber));
committingPrNum =
await webservices.GetPrNumber(task.InternalName, task.Manifest!.Plugin.Commit)
?? throw new Exception($"No PR number for commit ({task.InternalName}, {task.Manifest.Plugin.Commit})");
taskToPrNumber.Add(task, committingPrNum.Value);

if (string.IsNullOrEmpty(changelog))
{
changelog = await gitHubApi!.GetIssueBody(committingPrNum.Value);
}

reviewer = await gitHubApi!.GetReviewer(committingPrNum.Value);
Log.Information("Reviewer for {InternalName} ({PrNum}): {Reviewer}", task.InternalName, committingPrNum.Value, reviewer);
}

var buildResult = await buildProcessor.ProcessTask(task, mode == ModeOfOperation.Commit, changelog, tasks);
var buildResult = await buildProcessor.ProcessTask(task, mode == ModeOfOperation.Commit, changelog, reviewer, tasks);
allResults.Add(buildResult);

var mainDiffUrl = buildResult.Diff?.HosterUrl ?? buildResult.Diff?.RegularDiffLink;
Expand All @@ -309,7 +327,6 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,
Log.Information("Built: {Name} - {Sha} - {DiffUrl} +{LinesAdded} -{LinesRemoved}", task.InternalName,
task.Manifest.Plugin.Commit, mainDiffUrl ?? "null", buildResult.Diff?.LinesAdded ?? -1, buildResult.Diff?.LinesRemoved ?? -1);


var linesAddedText = buildResult.Diff?.LinesAdded == null ? "?" : buildResult.Diff.LinesAdded.ToString();
var prevVersionText = string.IsNullOrEmpty(buildResult.PreviousVersion)
? string.Empty
Expand Down Expand Up @@ -339,9 +356,11 @@ static async Task Main(DirectoryInfo outputFolder, DirectoryInfo manifestFolder,
}
}

if (!string.IsNullOrEmpty(prNumber) && mode == ModeOfOperation.PullRequest)
if (mode == ModeOfOperation.PullRequest)
{
await webservices.RegisterPrNumber(task.InternalName, task.Manifest.Plugin.Commit,
prNumber);
prNumber ?? throw new Exception("No PR number"));
}

if (buildResult.Diff != null)
{
Expand All @@ -362,27 +381,22 @@ await webservices.RegisterPrNumber(task.InternalName, task.Manifest.Plugin.Commi

if (mode == ModeOfOperation.Commit)
{
int? prInt = null;
if (int.TryParse(
await webservices.GetPrNumber(task.InternalName, task.Manifest.Plugin.Commit),
out var commitPrNum))
if (committingPrNum == null)
throw new Exception("No PR number for commit");

// Let's try getting the changelog again here in case we didn't get it the first time around
if (string.IsNullOrEmpty(changelog) && repoName != null &&
gitHubApi != null)
{
// Let's try again here in case we didn't get it the first time around
if (string.IsNullOrEmpty(changelog) && repoName != null &&
gitHubApi != null)
{
changelog = await gitHubApi.GetIssueBody(commitPrNum);
}

prInt = commitPrNum;
changelog = await gitHubApi.GetIssueBody(committingPrNum.Value);
}

await webservices.StagePluginBuild(new WebServices.StagedPluginInfo
{
InternalName = task.InternalName,
Version = buildResult.Version!,
Dip17Track = task.Channel,
PrNumber = prInt,
PrNumber = committingPrNum.Value,
Changelog = changelog,
IsInitialRelease = task.IsNewPlugin,
DiffLinesAdded = buildResult.Diff?.LinesAdded,
Expand Down Expand Up @@ -465,7 +479,10 @@ string ReplaceDiscordEmotes(string text)

if (mode == ModeOfOperation.PullRequest)
{
var existingMessages = await webservices.GetMessageIds(prNumber!);
if (prNumber == null)
throw new Exception("PR number not set");

var existingMessages = await webservices.GetMessageIds(prNumber.Value);
var alreadyPosted = existingMessages.Length > 0;

var links =
Expand All @@ -477,10 +494,8 @@ string ReplaceDiscordEmotes(string text)
if (!anyTried)
commentText =
"⚠️ No builds attempted! This probably means that your owners property is misconfigured.";

var parsedPrNum = int.Parse(prNumber!);

var crossOutTask = gitHubApi?.CrossOutAllOfMyComments(parsedPrNum);

var crossOutTask = gitHubApi?.CrossOutAllOfMyComments(prNumber.Value);

var anyComments = true;
if (crossOutTask != null)
Expand Down Expand Up @@ -556,7 +571,7 @@ string ReplaceDiscordEmotes(string text)
needsText = hiddenText;
}

var commentTask = gitHubApi?.AddComment(parsedPrNum,
var commentTask = gitHubApi?.AddComment(prNumber.Value,
commentText + mergeTimeText + "\n\n" + buildsMd + needsText + "\n##### " + links);

if (commentTask != null)
Expand All @@ -569,7 +584,7 @@ string ReplaceDiscordEmotes(string text)
{
hookTitle += " created";

var prDesc = await gitHubApi!.GetIssueBody(parsedPrNum);
var prDesc = await gitHubApi!.GetIssueBody(prNumber.Value);
if (!string.IsNullOrEmpty(prDesc))
buildInfo += $"```\n{prDesc}\n```\n";
}
Expand All @@ -592,14 +607,14 @@ string ReplaceDiscordEmotes(string text)
var id = await publicChannelWebhook.Send(ok ? Color.Purple : Color.Red,
$"{buildInfo}\n\n{links} - [PR](https://github.com/goatcorp/DalamudPluginsD17/pull/{prNumber})",
hookTitle, ok ? "Accepted" : "Rejected");
await webservices.RegisterMessageId(prNumber!, id);
await webservices.RegisterMessageId(prNumber.Value, id);

if (gitHubApi != null)
await gitHubApi.SetPrLabels(parsedPrNum, prLabels);
await gitHubApi.SetPrLabels(prNumber.Value, prLabels);

if (prLabels.HasFlag(GitHubApi.PrLabel.NewPlugin) && gitHubApi != null)
{
await DoPacRoundRobinAssign(gitHubApi, parsedPrNum);
await DoPacRoundRobinAssign(gitHubApi, prNumber.Value);
}
}

Expand All @@ -618,13 +633,9 @@ string ReplaceDiscordEmotes(string text)
if (!buildResult.Success && !aborted)
continue;

var resultPrNum =
await webservices.GetPrNumber(buildResult.Task.InternalName, buildResult.Task.Manifest!.Plugin.Commit);
if (resultPrNum == null)
if (!taskToPrNumber.TryGetValue(buildResult.Task, out var resultPrNum))
{
Log.Warning("No PR for {InternalName} - {Version}", buildResult.Task.InternalName,
buildResult.Version);
continue;
throw new Exception($"No PR number for commit {buildResult.Task.InternalName} - {buildResult.Task.Manifest!.Plugin.Commit}");
}

try
Expand Down
Loading

0 comments on commit c9656a0

Please sign in to comment.