Skip to content

Commit

Permalink
5 add option to only add comments on files in the pr (#14)
Browse files Browse the repository at this point in the history 
This merge includes significant enhancements and a critical bug fix:

- We've added scoping to pull requests in the task. This functionality provides greater efficiency and accuracy when dealing with large scopes. It ensures that the task is focusing solely on the relevant files within a pull request, greatly improving processing time and reliability.

- A fix has been implemented for an issue that was affecting the correct setting of the PR status. With this fix, the PR status accurately reflects the current state of the pull request, thus streamlining the PR management process and avoiding potential miscommunications.

These changes contribute to a robust and reliable system, proving vital for effective pull request management.

---------

Co-authored-by: Alexander Jeckmans <ajeckmans@users.noreply.github.com>
  • Loading branch information
@ajeckmans
ajeckmans and ajeckmans committed Oct 9, 2023
1 parent fe44ae5 commit 050e6e3
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 32 deletions.
9 changes: 5 additions & 4 deletions src/format-check/scripts/TaskParameters.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
export interface TaskParameters {
solutionPath: string;
includePath: string | undefined;
excludePath: string | undefined;
statusCheck: boolean;
failOnFormattingErrors: boolean;
includePath: string | undefined;
solutionPath: string;
scopeToPullRequest: boolean;
statusCheck: boolean;
statusCheckContext: {
name: string;
genre: string;
name: string;
};
token: string | undefined;
}
70 changes: 53 additions & 17 deletions src/format-check/scripts/format-check.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import * as fs from 'fs';
import dotenv from 'dotenv';
import * as azdev from "azure-devops-node-api";
import * as gi from "azure-devops-node-api/interfaces/GitInterfaces";
import {CommentThreadStatus, VersionControlChangeType} from "azure-devops-node-api/interfaces/GitInterfaces";
import {TaskParameters} from './TaskParameters';
import {EnvVariables} from './EnvVariables';

import {FormatReports} from './format-report.interface';
import {IGitApi} from "azure-devops-node-api/GitApi";
import {GitPullRequestIteration} from "azure-devops-node-api/interfaces/GitInterfaces";

const commentPreamble = '[DotNetFormatTask][Automated]';

Expand All @@ -33,10 +33,10 @@ async function main() {
}

// Run the format check
const reports = runFormatCheck(taskParams);
const reports = await runFormatCheck(gitApi, envVars, taskParams);

// Check the format and set PR according to the result
var shouldFail = await checkFormatAndSetPR(gitApi, reports, envVars, taskParams);
let shouldFail = await checkFormatAndSetPR(gitApi, reports, envVars, taskParams);

if (shouldFail) {
console.log("Format check task failed.");
Expand Down Expand Up @@ -69,6 +69,7 @@ function getTaskParameters(envVars: EnvVariables): TaskParameters {
name: process.env.INPUT_STATUSCHECKNAME,
genre: process.env.INPUT_STATUSCHECKGENRE,
},
scopeToPullRequest: process.env.INPUT_SCOPETOPULLREQUEST === 'true',
token: process.env.INPUT_PAT || envVars.token
};

Expand All @@ -80,6 +81,7 @@ function getTaskParameters(envVars: EnvVariables): TaskParameters {
console.log(`Fail On Formatting Errors: ${params.failOnFormattingErrors}`);
console.log(`Status Check Name: ${params.statusCheckContext.name}`);
console.log(`Status Check Genre: ${params.statusCheckContext.genre}`);
console.log(`Scope To Pull Request: ${params.scopeToPullRequest}`);

return params;
}
Expand All @@ -102,7 +104,7 @@ function getEnvVariables(): EnvVariables {
return envVars;
}

function runFormatCheck(taskParams: TaskParameters) {
async function runFormatCheck(gitApi: IGitApi, envVars: EnvVariables, taskParams: TaskParameters) {
const reportPath = "format-report.json";

validateSolutionPath(taskParams.solutionPath, reportPath);
Expand All @@ -115,13 +117,12 @@ function runFormatCheck(taskParams: TaskParameters) {

try {
execSync(formatCmd, {stdio: 'inherit'});
execSync(formatCmd, {stdio: 'pipe'});
} catch (error) {
handleDotnetFormatError(error, reportPath);
}

console.log("Dotnet format command completed.");
return loadErrorReport(reportPath);
return await loadErrorReport(gitApi, envVars, reportPath, taskParams.scopeToPullRequest);
} catch (error) {
console.error(`Dotnet format task failed with error ${error}`);
process.exit(1);
Expand Down Expand Up @@ -158,9 +159,35 @@ function handleDotnetFormatError(error: any, reportPath: string) {
}
}

function loadErrorReport(reportPath: string) {
async function loadErrorReport(gitApi: IGitApi, envVars: EnvVariables, reportPath: string, scopeToPullRequest: boolean) {
console.log("Loading error report.");
return JSON.parse(fs.readFileSync(reportPath, 'utf8')) as FormatReports;
let report = JSON.parse(fs.readFileSync(reportPath, 'utf8')) as FormatReports;

// normalize filepath
report = report.map(r => ({...r, FilePath: r.FilePath.replace(`${process.env.BUILD_SOURCESDIRECTORY}`, '')}))

if (scopeToPullRequest) {
console.log("Scoping to Pull Request.");
const filesInPR = await getPullRequestFiles(gitApi, envVars);
report = report.filter(r => filesInPR.includes(r.FilePath));
}

return report;
}

async function getPullRequestFiles(gitApi: IGitApi, envVars: EnvVariables) {
console.log("Getting the PR commits...");
const pullRequestCommits = await gitApi.getPullRequestCommits(envVars.repoId, envVars.pullRequestId, envVars.projectId);
const latestCommit = pullRequestCommits.slice(-1)[0];
console.log("latest commit: " + latestCommit.commitId)

const changes = await gitApi.getChanges(latestCommit.commitId, envVars.repoId, envVars.projectId);
let files = changes.changes
.filter(change => change.changeType !== VersionControlChangeType.Delete)
.map(change => change.item.path);

console.log("Commit file changed obtained: ", files);
return files;
}

async function checkFormatAndSetPR(gitApi: IGitApi, reports: FormatReports, envVars: EnvVariables, taskParams: TaskParameters) {
Expand Down Expand Up @@ -195,7 +222,7 @@ async function checkFormatAndSetPR(gitApi: IGitApi, reports: FormatReports, envV
comments: [comment],
status: gi.CommentThreadStatus.Active,
threadContext: {
filePath: report.FilePath.replace(`${process.env.BUILD_SOURCESDIRECTORY}`, ''),
filePath: report.FilePath,
rightFileStart: {line: change.LineNumber, offset: change.CharNumber},
rightFileEnd: {line: change.LineNumber, offset: change.CharNumber + 1}
}
Expand All @@ -209,13 +236,15 @@ async function checkFormatAndSetPR(gitApi: IGitApi, reports: FormatReports, envV
await markResolvedThreadsAsClosed(existingThreads, activeIssuesContent, gitApi, envVars);

// Set PR status and fail the task if necessary
var shouldFail = await setPRStatusAndFailTask(activeIssuesContent.length > 0, gitApi, envVars, taskParams);
return shouldFail;
return await setPRStatusAndFailTask(reports.length > 0, gitApi, envVars, taskParams);
}

async function markResolvedThreadsAsClosed(existingThreads: gi.GitPullRequestCommentThread[], activeIssuesContent: string[], gitApi: IGitApi, envVars: EnvVariables) {
for (const existingThread of existingThreads.filter(thread => thread.comments.some(comment => comment.content?.startsWith(commentPreamble)))) {
console.log("Processing the existing thread");
if (existingThread.status === CommentThreadStatus.Closed) {
continue;
}
const threadContent = existingThread.comments[0]?.content;
if (!activeIssuesContent.includes(threadContent)) {
console.log("Closing resolved thread.");
Expand All @@ -228,15 +257,17 @@ async function markResolvedThreadsAsClosed(existingThreads: gi.GitPullRequestCom
}

async function setPRStatusAndFailTask(formatIssuesExist: boolean, gitApi: IGitApi, envVars: EnvVariables, taskParams: TaskParameters) {
if (taskParams.statusCheck) {
await updatePullRequestStatus(gitApi, envVars, taskParams, formatIssuesExist ? gi.GitStatusState.Failed : gi.GitStatusState.Succeeded);
}

if (formatIssuesExist && taskParams.failOnFormattingErrors) {
console.log(`##vso[task.complete result=Failed;]Code format is incorrect.`);
if (taskParams.statusCheck) {
await updatePullRequestStatus(gitApi, envVars, taskParams, gi.GitStatusState.Failed);
}
return true;
} else {
console.log(`##vso[task.complete result=Succeeded;]Code format is correct.`);
if (taskParams.statusCheck) {
await updatePullRequestStatus(gitApi, envVars, taskParams, gi.GitStatusState.Succeeded);
}
return false;
}
}
Expand All @@ -258,8 +289,8 @@ async function updatePullRequestStatus(gitApi: IGitApi, envVars: EnvVariables, t
const logMsg = `Setting status check '${taskParams.statusCheckContext.genre}\\${taskParams.statusCheckContext.name}' to: ${gi.GitStatusState[status]}`;
console.log(logMsg);

const iterations: GitPullRequestIteration[] = await gitApi.getPullRequestIterations(envVars.repoId, envVars.pullRequestId, envVars.projectId, false);
let iterationId = Math.max(...iterations.map(iteration => iteration.id));
const iterationId = await getLastPullRequestIteration(gitApi, envVars);

const prStatus: gi.GitPullRequestStatus = {
context: taskParams.statusCheckContext,
state: status,
Expand All @@ -269,6 +300,11 @@ async function updatePullRequestStatus(gitApi: IGitApi, envVars: EnvVariables, t
await gitApi.createPullRequestStatus(prStatus, envVars.repoId, envVars.pullRequestId);
}

async function getLastPullRequestIteration(gitApi: IGitApi, envVars: EnvVariables) {
const pullRequestIterations = await gitApi.getPullRequestIterations(envVars.repoId, envVars.pullRequestId, envVars.projectId, true);
return pullRequestIterations[pullRequestIterations.length - 1].id;
}

// Call these functions in your main function and do your own error handling
main().catch(error => {
console.error(error);
Expand Down
31 changes: 20 additions & 11 deletions src/format-check/task.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
"Node10": {
"target": "scripts/format-check.js",
"inputs": {
"solutionPath": "INPUT_SOLUTIONPATH",
"includePath": "INPUT_INCLUDEPATH",
"excludePath": "INPUT_EXCLUDEPATH",
"failOnFormattingErrors": "INPUT_FAILONFORMATTINGERRORS",
"statusCheck": "INPUT_STATUSCHECK",
"statusCheckName": "INPUT_STATUSCHECKNAME",
"statusCheckGenre": "INPUT_STATUSCHECKGENRE"
"SolutionPath": "INPUT_SOLUTIONPATH",
"IncludePath": "INPUT_INCLUDEPATH",
"ExcludePath": "INPUT_EXCLUDEPATH",
"FailOnFormattingErrors": "INPUT_FAILONFORMATTINGERRORS",
"StatusCheck": "INPUT_STATUSCHECK",
"StatusCheckName": "INPUT_STATUSCHECKNAME",
"StatusCheckGenre": "INPUT_STATUSCHECKGENRE",
"ScopeToPullRequest": "INPUT_SCOPETOPULLREQUEST"
}
}
},
Expand Down Expand Up @@ -45,36 +46,44 @@
"helpMarkDown": "Specify the path to be excluded."
},
{
"name": "failOnFormattingErrors",
"name": "FailOnFormattingErrors",
"type": "boolean",
"label": "Fail on Formatting Errors",
"defaultValue": "true",
"required": false,
"helpMarkDown": "If set to false, the task will NOT fail when formatting errors are found."
},
{
"name": "statusCheck",
"name": "StatusCheck",
"type": "boolean",
"label": "Set PR Status Check",
"defaultValue": "false",
"required": false,
"helpMarkDown": "If set to true, the task will set a status check in the PR when formatting errors are found."
},
{
"name": "statusCheckName",
"name": "StatusCheckName",
"type": "string",
"label": "Status Check Name",
"defaultValue": "format check",
"required": false,
"helpMarkDown": "Name of the status check in the PR for formatting errors. This input is only considered if 'Set PR Status Check' is true."
},
{
"name": "statusCheckGenre",
"name": "StatusCheckGenre",
"type": "string",
"label": "Status Check Genre",
"defaultValue": "formatting",
"required": false,
"helpMarkDown": "Genre of the status check in the PR for formatting errors. This input is only considered if 'Set PR Status Check' is true."
},
{
"name": "ScopeToPullRequest",
"type": "boolean",
"label": "Scope to Pull Request",
"defaultValue": "false",
"required": false,
"helpMarkDown": "When checked, the task only considers failures for files in the pull request. E.g., only add comments to files on the PR and fail only for failures for comments in the PR."
}
]
}

0 comments on commit 050e6e3

Please sign in to comment.