Skip to content

fix(ci): improve changed file detection in linter script #8968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shivanee-p wants to merge 3 commits into
base: main
Choose a base branch
from
+20 −3
Open

fix(ci): improve changed file detection in linter script #8968

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46f4da7
fix(ci): improve changed file detection in linter script
shivanee-p Jul 24, 2026
480b5d0
fix(ci): use explicit refspec when fetching base branch
shivanee-p Jul 24, 2026
fd5e150
fix(linter): use namespace import for typescript ESM compatibility
shivanee-p Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions bin/linter.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {existsSync} from 'fs';
import path from 'path';
import {promisify} from 'util';
import {ESLint} from 'eslint';
import ts from 'typescript';
import * as ts from 'typescript';

// --- Globals & Promisified API Wrappers ---
const execFileAsync = promisify(execFile);
Expand Down Expand Up @@ -68,11 +68,28 @@ function runGit(args, options = {}) {
*/
function getChangedFiles() {
const base = process.env.GITHUB_BASE_REF || 'main';

@westarle westarle Jul 29, 2026

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.

I think if the script is running in CI, it should fail here if GITHUB_BASE_REF isn't set.


// Attempt to fetch the base branch and set origin/${base} so it doesn't compare against itself

@westarle westarle Jul 29, 2026

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.

I'd prefer to make the script hermetic and have our CI configuration produce the right environment for it to be effective. Can we avoid mutation in the script and have the workflow use actions/checkout

if (process.env.GITHUB_BASE_REF) {
try {
runGit([
'fetch',
'origin',
`+refs/heads/${base}:refs/remotes/origin/${base}`,
'--depth=1',

@westarle westarle Jul 29, 2026

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 main has moved and the branch has moved, the common ancestor might be higher than 1; worth considering here or as fetch-depth in actions/checkout.

]);
} catch {
// Continue if network fetch fails or remote does not exist
}
}

const refsToTry = [

@westarle westarle Jul 29, 2026

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.

It might be simpler to say:

  1. if it's in CI then use exactly process.env.GITHUB_BASE_REF or fail
  2. if the local branch is "main" then use HEAD~1
  3. for other local branches, use upstream/main, origin/main and then main and print a line that says what we chose.

`origin/${base}...HEAD`,
`${base}...HEAD`,
`upstream/${base}...HEAD`,
`origin/${base}`,
base,
`upstream/${base}`,
`origin/${base}`,
'FETCH_HEAD',
'HEAD~1',
'HEAD^',
];
Expand Down
Loading