Skip to content

Commit

Permalink
clean up split suggestion server strict sloc function
Browse files Browse the repository at this point in the history
Summary:
Context: This is a series of diff where we clean up the ISL Split Suggestions Experiment.

This is probably the last diff for this clean up.
I'm removing the strict SLOC that's returned by the server since it is no longer in use.

Details:
https://fb.workplace.com/groups/codeproflowexperiments/permalink/861491969380571/

Reviewed By: benmonro

Differential Revision: D66733720

fbshipit-source-id: ca9465d95fa67c4479270978dd4a5ee5bf2bb68e
  • Loading branch information
Jack Huang authored and facebook-github-bot committed Dec 4, 2024
1 parent 8b604a2 commit 80c0aaa
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 68 deletions.
62 changes: 12 additions & 50 deletions addons/isl-server/src/Repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ export class Repository {
ctx: RepositoryContext,
hash: Hash,
excludedFiles: string[],
): Promise<{sloc: number; strictSloc: number}> {
): Promise<number | undefined> {
const exclusions = excludedFiles.flatMap(file => [
'-X',
absolutePathForFileInRepo(file, this) ?? file,
Expand All @@ -1200,24 +1200,18 @@ export class Repository {
).stdout;

const sloc = this.parseSlocFrom(output);
const strictSloc = this.parseStrictSlocFrom(output);

ctx.logger.info(
'Fetched SLOC for commit:',
hash,
output,
`SLOC: ${sloc} Strict SLOC: ${strictSloc}`,
);
return {sloc, strictSloc};
ctx.logger.info('Fetched SLOC for commit:', hash, output, `SLOC: ${sloc}`);
return sloc;
}

public async fetchPendingAmendSignificantLinesOfCode(
ctx: RepositoryContext,
hash: Hash,
includedFiles: string[],
): Promise<{sloc: number; strictSloc: number}> {
): Promise<number | undefined> {
if (includedFiles.length === 0) {
return {sloc: 0, strictSloc: 0}; // don't bother running sl diff if there are no files to include
return undefined;
}
const inclusions = includedFiles.flatMap(file => [
'-I',
Expand All @@ -1233,29 +1227,22 @@ export class Repository {
).stdout;

if (output.trim() === '') {
return {sloc: 0, strictSloc: 0};
return undefined;
}

const sloc = this.parseSlocFrom(output);
const strictSloc = this.parseStrictSlocFrom(output);

ctx.logger.info(
'Fetched Pending AMEND SLOC for commit:',
hash,
output,
`SLOC: ${sloc} Strict SLOC: ${strictSloc}`,
);

return {sloc, strictSloc};
ctx.logger.info('Fetched Pending AMEND SLOC for commit:', hash, output, `SLOC: ${sloc}`);
return sloc;
}

public async fetchPendingSignificantLinesOfCode(
ctx: RepositoryContext,
hash: Hash,
includedFiles: string[],
): Promise<{sloc: number; strictSloc: number}> {
): Promise<number | undefined> {
if (includedFiles.length === 0) {
return {sloc: 0, strictSloc: 0}; // don't bother running sl diff if there are no files to include
return undefined; // don't bother running sl diff if there are no files to include
}
const inclusions = includedFiles.flatMap(file => [
'-I',
Expand All @@ -1271,15 +1258,9 @@ export class Repository {
).stdout;

const sloc = this.parseSlocFrom(output);
const strictSloc = this.parseStrictSlocFrom(output);

ctx.logger.info(
'Fetched Pending SLOC for commit:',
hash,
output,
`SLOC: ${sloc} Strict SLOC: ${strictSloc}`,
);
return {sloc, strictSloc};
ctx.logger.info('Fetched Pending SLOC for commit:', hash, output, `SLOC: ${sloc}`);
return sloc;
}

private parseSlocFrom(output: string) {
Expand All @@ -1293,25 +1274,6 @@ export class Repository {
return sloc;
}

private parseStrictSlocFrom(output: string) {
let sloc = 0;
const lines = output.trim().split('\n');

for (let i = 0; i < lines.length - 1; i++) {
const fileChange = lines[i];
const [path, lineChange] = fileChange.split('|');
const trimmedPath = path.trim();

if (!trimmedPath.includes('/__tests__/') && !trimmedPath.endsWith('.md')) {
const lineChangeMatch = lineChange.match(/\d+/);
const lineChangeCount = lineChangeMatch ? parseInt(lineChangeMatch[0]) : 0;
sloc += lineChangeCount;
}
}

return sloc;
}

public async getAllChangedFiles(ctx: RepositoryContext, hash: Hash): Promise<Array<ChangedFile>> {
const output = (
await this.runCommand(
Expand Down
6 changes: 3 additions & 3 deletions addons/isl-server/src/ServerToClientAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ export default class ServerToClientAPI {
type: 'fetchedPendingSignificantLinesOfCode',
requestId: data.requestId,
hash: data.hash,
result: {value: {linesOfCode: value.sloc, strictLinesOfCode: value.strictSloc}},
result: {value: value ?? 0},
});
})
.catch(err => {
Expand All @@ -614,7 +614,7 @@ export default class ServerToClientAPI {
this.postMessage({
type: 'fetchedSignificantLinesOfCode',
hash: data.hash,
result: {value: {linesOfCode: value.sloc, strictLinesOfCode: value.strictSloc}},
result: {value: value ?? 0},
});
})
.catch(err => {
Expand All @@ -635,7 +635,7 @@ export default class ServerToClientAPI {
type: 'fetchedPendingAmendSignificantLinesOfCode',
requestId: data.requestId,
hash: data.hash,
result: {value: {linesOfCode: value.sloc, strictLinesOfCode: value.strictSloc}},
result: {value: value ?? 0},
});
})
.catch(err => {
Expand Down
2 changes: 1 addition & 1 deletion addons/isl-server/src/__tests__/Repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ www/flib/intern/entity/diff/EntPhabricatorDiffSchema.php

const ejecaSpy = mockEjeca([[/^sl diff/, () => ({stdout: EXAMPLE_DIFFSTAT})]]);
const results = repo.fetchSignificantLinesOfCode(ctx, 'abcdef', ['generated.file']);
await expect(results).resolves.toEqual({sloc: 45, strictSloc: 45});
await expect(results).resolves.toEqual(45);
expect(ejecaSpy).toHaveBeenCalledWith(
'sl',
expect.arrayContaining([
Expand Down
14 changes: 5 additions & 9 deletions addons/isl/src/sloc/useFetchSignificantLinesOfCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ async function fetchSignificantLinesOfCode(
const slocData = await serverAPI
.nextMessageMatching('fetchedSignificantLinesOfCode', message => message.hash === commit.hash)
.then(result => ({
sloc: result.result.value?.linesOfCode,
strictSloc: result.result.value?.strictLinesOfCode,
sloc: result.result.value,
}));

return slocData;
Expand Down Expand Up @@ -125,7 +124,7 @@ const fetchPendingAmendSloc = async (
}

if (filteredFiles.length === 0) {
return {sloc: 0, strictSloc: 0};
return {sloc: 0};
}

//the calculation here is a bit tricky but in nutshell it is:
Expand All @@ -151,8 +150,7 @@ const fetchPendingAmendSloc = async (
message => message.requestId === requestId && message.hash === commit.hash,
)
.then(result => ({
sloc: result.result.value?.linesOfCode,
strictSloc: result.result.value?.strictLinesOfCode,
sloc: result.result.value,
}));

if (unselectedCommittedSlocInfo === undefined) {
Expand All @@ -165,7 +163,6 @@ const fetchPendingAmendSloc = async (

const slocInfo = {
sloc: (unselectedCommittedSlocInfo.sloc ?? 0) + (pendingLoc.sloc ?? 0),
strictSloc: (unselectedCommittedSlocInfo.strictSloc ?? 0) + (pendingLoc.strictSloc ?? 0),
};

return slocInfo;
Expand Down Expand Up @@ -211,7 +208,7 @@ const fetchPendingSloc = async (
}

if (filteredFiles.length === 0) {
return {sloc: 0, strictSloc: 0};
return {sloc: 0};
}

serverAPI.postMessage({
Expand All @@ -227,8 +224,7 @@ const fetchPendingSloc = async (
message => message.requestId === requestId && message.hash === commit.hash,
)
.then(result => ({
sloc: result.result.value?.linesOfCode,
strictSloc: result.result.value?.strictLinesOfCode,
sloc: result.result.value,
}));

return pendingLocData;
Expand Down
8 changes: 3 additions & 5 deletions addons/isl/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,6 @@ export type StableInfo = {
export type SlocInfo = {
/** Significant lines of code for commit */
sloc: number | undefined;
/** Significant lines of code for commit (filtering out test and markdown files) */
strictSloc: number | undefined;
};

export type CommitInfo = {
Expand Down Expand Up @@ -950,19 +948,19 @@ export type ServerToClientMessage =
| {
type: 'fetchedSignificantLinesOfCode';
hash: Hash;
result: Result<{linesOfCode: number; strictLinesOfCode: number}>;
result: Result<number>;
}
| {
type: 'fetchedPendingSignificantLinesOfCode';
requestId: number;
hash: Hash;
result: Result<{linesOfCode: number; strictLinesOfCode: number}>;
result: Result<number>;
}
| {
type: 'fetchedPendingAmendSignificantLinesOfCode';
requestId: number;
hash: Hash;
result: Result<{linesOfCode: number; strictLinesOfCode: number}>;
result: Result<number>;
};
export type Disposable = {
dispose(): void;
Expand Down

0 comments on commit 80c0aaa

Please sign in to comment.