Skip to content

fix: fixed getAuthors#1930

Open
Yurok868 wants to merge 2 commits into
masterfrom
fix/fix-getAuthors
Open

fix: fixed getAuthors#1930
Yurok868 wants to merge 2 commits into
masterfrom
fix/fix-getAuthors

Conversation

@Yurok868
Copy link
Copy Markdown
Contributor

fix: fixed getAuthors

@Yurok868 Yurok868 requested a review from a team as a code owner May 14, 2026 11:34
@Yurok868 Yurok868 requested review from separatrixxx and removed request for a team May 14, 2026 11:34
Comment thread src/extensions/arcadia-vcs/arc-client.ts
Comment thread src/extensions/arcadia-vcs/arc-client.ts
@Yurok868 Yurok868 force-pushed the fix/fix-getAuthors branch from d734c0c to fdd4767 Compare May 15, 2026 08:12
@Yurok868 Yurok868 force-pushed the fix/fix-getAuthors branch from fdd4767 to 695d4f5 Compare May 15, 2026 08:31
@sonarqubecloud
Copy link
Copy Markdown

async getAuthors(): Promise<Record<string, {login: string; commit: string}>> {
const base = await this.getBase();
const scopes = [base, ...(this.config.vcs.scopes || [])].map(normalizePath);
const handled = new Set<string>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you remove this behavior, several cases will break. Add tests for them

  1. getAuthors: one commit affects files in different scopes — we check that the author is set for paths from all scopes, not just the first one. This is the main bug that the PR is fixing. It is already in the PR.

  2. getContributors: one commit affects files in different scopes — similar to point 1, but for an accumulating array of contributors. It is already in the PR, but the mock for . is unrealistic — we need arc log . returned all commit paths, including those that would be included in the sub-scope.

  3. getContributors: one path in the overlapping scopes (. + docs) - arc log . and arc log docs both return a commit with the path docs/intro.md; we check that there is only one entry in the contributors array, not two. This is a regression caused by removing the handled Set from getContributors, and it is directly related to the changes in the PR.

shouldBeIgnored(ignore, {login: commit.login}) || handled.has(commit.sha);
handled.add(commit.sha);
const skip = shouldBeIgnored(ignore, {login: commit.login});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix: fixed getAuthors - it doesn't say anything, so I suggest something like fix(arcadia-vcs): credit authors on cross-scope commits in getAuthor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants