Skip to content

refactor(ng-dev/release): centralize and expand ignored paths for snapshot publishing#3557

Merged
alan-agius4 merged 1 commit intoangular:mainfrom
alan-agius4:sass-lock-file
Mar 17, 2026
Merged

refactor(ng-dev/release): centralize and expand ignored paths for snapshot publishing#3557
alan-agius4 merged 1 commit intoangular:mainfrom
alan-agius4:sass-lock-file

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

Centralizes the paths to exclude when detecting changes for snapshot publishing into a single constant. Additionally, adds pubspec.lock to the list of ignored files to avoid unnecessary snapshot releases when only lock files change.

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Mar 17, 2026
…pshot publishing

Centralizes the paths to exclude when detecting changes for snapshot
publishing into a single constant. Additionally, adds `pubspec.lock`
to the list of ignored files to avoid unnecessary snapshot releases
when only lock files change.
Copy link
Copy Markdown
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the snapshot publishing logic to centralize and expand the list of excluded file paths. The changes are well-implemented. I've suggested a minor improvement to move the new constant into the SnapshotPublisher class for better encapsulation, which requires a corresponding update to its usage.

Comment on lines +34 to 39
/**
* Paths to exclude from the snapshot commit.
*/
const PATHS_TO_EXCLUDE = ['**/MODULE.bazel.lock', '**/package-lock.json', '**/pubspec.lock'];

export class SnapshotPublisher {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For better encapsulation, consider moving PATHS_TO_EXCLUDE inside the SnapshotPublisher class as a private static readonly property. This constant is only used within this class, so this change would improve cohesion.

export class SnapshotPublisher {
  /**
   * Paths to exclude from the snapshot commit.
   */
  private static readonly PATHS_TO_EXCLUDE = ['**/MODULE.bazel.lock', '**/package-lock.json', '**/pubspec.lock'];

'.',
':(exclude)**/MODULE.bazel.lock',
':(exclude)**/package-lock.json',
...PATHS_TO_EXCLUDE.map((p) => `:(exclude)${p}`),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To align with moving PATHS_TO_EXCLUDE into the SnapshotPublisher class, please update its usage here to reference it as a static property.

              ...SnapshotPublisher.PATHS_TO_EXCLUDE.map((p) => `:(exclude)${p}`),

@alan-agius4 alan-agius4 merged commit b55a8a5 into angular:main Mar 17, 2026
13 checks passed
@alan-agius4
Copy link
Copy Markdown
Contributor Author

This PR was merged into the repository. The changes were merged into the following branches:

@alan-agius4 alan-agius4 deleted the sass-lock-file branch March 17, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants