[DX-2353] feat(ci): lint only changed modules

[erikburt]

Changes

• Use changed-modules-go to determine which modules have changed based on the triggering event
• Fix matrix strategy definition

Testing

This PR - change no modules - https://github.com/smartcontractkit/chainlink-common/actions/runs/22467329645/job/65076281025?pr=1858

• TEST - feat(ci): lint only changed modules #1859 - change 3 modules - https://github.com/smartcontractkit/chainlink-common/actions/runs/22467535430/job/65076946312?pr=1859

---

DX-2353

[github-actions]

✅ API Diff Results - No breaking changes

---

📄 View full apidiff report

[Copilot]

Pull request overview

Updates the Go lint GitHub Actions workflow to reduce CI work by computing which Go modules were affected by a PR and linting only those modules, while also adjusting the matrix strategy behavior.

Changes:

• Replace “lint all modules” detection logic with smartcontractkit/.github/actions/changed-modules-go output.
• Gate the lint matrix job on whether any modules were detected as changed.
• Adjust matrix behavior to avoid fail-fast, and deepen checkout history for lint runs.

Comments suppressed due to low confidence (2)

.github/workflows/golangci_lint.yml:36

• The comment says we “Avoid running in merge queue”, but only lint-module is gated; detect-modules will still run on merge_group events because the workflow is triggered for merge_group. If the intent is to avoid merge queue entirely, consider also gating detect-modules (or removing merge_group from the workflow trigger) so the workflow doesn’t execute any jobs in merge queue runs.

    # Avoid running in merge queue since we run in PR's and have an optional
    # label to skip lint issues on a PR which would be ignored in the merge queue.
    if: ${{ needs.detect-modules.outputs.modules != '[]' && github.event_name != 'merge_group' }}

.github/workflows/golangci_lint.yml:24

• This comment references scheduled/workflow_dispatch, but this workflow currently only runs on pull_request and merge_group. Either add those triggers or adjust the comment so it describes the actual events that can hit this workflow.

          # when scheduled/workflow_dispatch, run against all modules
          no-change-behaviour: all

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jmank88]

Is this sufficient? Or do we need to be sensitive to embedded file changes too? Usually they are significant, but maybe not as much in this case 🤔

[erikburt]

I think this is fine. Does golangci-lint, have rules for embedded files?

[erikburt]

Anything that gets linted by golangci-lint and/or can produce a linting error should be within this scope.

[jmank88]

IIUC the linter works by doing some kind of partial compilation, or equivalent introspection via tooling, which may understand something about embedded files. I am not certain though. I don't see any rules for embedded files specifically. Maybe we should do some experimenting?

[erikburt]

I looked into it, and couldn't find any indication that anything other than *.go files are linted. Only the existence of embedded files will be checked via the compiler and that should fail all the build/test pipelines if it is missing.

Seems like static analysis on any other file types need their own linting tools.

---

One thing we can do, is run cron'd lints on all modules, and if any new linting errors arise that should've been caught in a PR, we can investigate why it got past the filter?

[erikburt]

One gap I've noticed based on this conversation, is changed-modules-go should have a set of globs/files that should cause it to label all modules as changed. For example, if the workflow, or the lint config was modified, then all modules should be linted. Going to tackle this in a couple weeks.

I don't think this should be a blocker for this PR though.

[erikburt]

scheduled linting: 378cf39

[kalverra]

@jmank88 golangci-lint only does some basic package loading. This approach of only focusing on Go files does leave a tiny gap for embedded files, but the consequences are small, and should be caught by other methods.

1. If you rename x.txt to y.txt but forget to update it in the Go file, the linter would catch this. But so would anything else that needs to do a basic build of the package, like go test.
2. Nope, I was wrong. See experiment below. If you embed a file as a string (//go:embed x.txt var content string), certain linters (like gocritic or revive) might analyze how that variable is used. If the content of x.txt changes the length or structure of that string, it could trigger "magic number" or "complexity" warnings elsewhere in your code.
3. Nope, I was wrong. See experiment below. If you have some custom linters checking other things around these embedded values, then maybe there are other issues slipping through.

I ran a quick experiment to check this

//go:embed regex.txt
var pattern string
var re = regexp.MustCompile(pattern)

func main() {
	fmt.Println(re.MatchString("hello"))
}

1. Changing to an invalid file //go:embed not-real.txt and running golangci-lint threw a typecheck linting error. So linting would catch it, but so does build, run, and test.
2. Changing the contents of regex.txt to invalid). golangci-lint and build did not catch this, but test and run did.

So, if you renamed an embedded file x.txt -> y.txt and failed to update it in the Go code, this lint step would not be able to catch it due to file filtering. But if you're regularly running test on the same changes, you're not missing anything. This seems like an excessively minor issue, but if you're worried, I only see a few options:

1. Forget the filter and lint everything every time (I'm guessing that's too expensive for this repo).
2. If you have a directory or file pattern we can add to the list (e.g., assets/ or *.txt), then that helps things a bit.
3. @erikburt's fix to run lints on cron jobs. It will let the issues through PRs, but they'll eventually get caught, and you can better pinpoint where.
4. Or you could do effectively the same thing as the cron job, only on pushes to the main branch.

If 3 seems reasonable to you, this should be good to go.

[kalverra]

I updated my comment above after running some experiments. @jmank88 hopefully this answers your question and we can merge.