feat: run lint only on affected packages

[davidcavazos]

Description

Fixes #

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

• I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
• Tests pass: npm test (see Testing)
• Lint pass: npm run lint (see Style)
• These samples need a new API enabled in testing projects to pass (let us know which ones)
• These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
• This pull request is from a branch created directly off of GoogleCloudPlatform/nodejs-docs-samples. Not a fork.
• This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
• This sample adds a new sample directory, and I created GitHub Actions workflow for this sample
• This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
• Please merge this PR for me once it is approved

[gemini-code-assist]

Hello @davidcavazos, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request modifies the CI workflow to run linting only on affected packages. It updates the .github/workflows/custard-ci.yaml file to rename the nodejs-lint job to lint, adds a dependency on the affected job, and modifies the linting step to use a GitHub script that determines the affected paths and runs npx gts lint on each of them. The script captures and logs the output of the linting command for each path and exits with an error if any of the linting commands fail.

Highlights

• Linting Optimization: The pull request optimizes the linting process by running it only on packages that have been affected by the changes.
• Workflow Modification: The CI workflow file (.github/workflows/custard-ci.yaml) is updated to implement the selective linting.
• GitHub Script Usage: A GitHub script is used to dynamically determine the affected paths and execute the linting command.

Changelog

• .github/workflows/custard-ci.yaml
  • Renamed the nodejs-lint job to lint.
  • Added a needs: affected dependency to the lint job.
  • Replaced the simple make lint command with a GitHub script.
  • The GitHub script determines affected paths using the output of the affected job.
  • The GitHub script executes npx gts lint on each affected path.
  • The GitHub script captures and logs the output of each linting command.
  • The GitHub script exits with an error if any linting command fails.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

The linter runs, a watchful eye,
On code that's pushed, reaching high.
But why check all,
When some stand tall,
And changes touch but paths nearby?

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request introduces a significant improvement by running linting only on affected packages, which can substantially reduce CI execution time. The changes are well-structured and the use of github-script action provides flexibility in handling the linting process. However, there are a few areas that could be improved for better maintainability and error reporting.

Summary of Findings

• Job Naming Convention: The job name nodejs-lint was renamed to lint. While this change itself isn't problematic, it's worth ensuring that this renaming aligns with the overall workflow naming conventions and doesn't introduce any unintended consequences in other parts of the CI/CD pipeline.
• Error Handling in Linting Script: The linting script captures stdout and stderr when linting fails, which is good for debugging. However, consider adding a mechanism to surface these logs more prominently in the CI output, perhaps by using annotations or a dedicated log aggregation service.
• Missing Error Message: The original workflow had an error message that was removed. It is important to keep this error message, or replace it with a new one, to ensure that the workflow fails as expected.

Merge Readiness

The pull request is almost ready for merging. The changes to run linting only on affected packages are a great improvement. However, the missing error message needs to be addressed before merging. I am unable to approve this pull request, and recommend that others review and approve this code before merging.

[davidcavazos]

Confirmed that lint now happens only on affected packages, and any package that fails lint causes the lint job to fail (see commit history above)

[briandorsey]

Overall, LGTM. Love this kind of incremental speed improvement!

[briandorsey]

question: Is the lint output on a successful pass useful? If so, consider printing stdout/stderr in both cases.

[davidcavazos]

That's a good question. I guess it depends on the lint tool. From the ones I've used they usually either don't have any output, or they just say something like "No issues found" or something like that. I was trying to keep the logs clean.

We can always add this functionality if we find a case where it's useful. I'm adding this as part of our reusable workflows, so there could be an option to show stdout/stderr from successful commands as well.