Skip to content

Conversation

paulRbr
Copy link
Member

@paulRbr paulRbr commented Jul 3, 2025

This commit is both a refactoring (to move the logic to apply overlays
inside the definition.extractDefinition to DRY the code and simplify
extracting an overlayed definition) and a new feature.

The new feature is to allow the --overlay flag on the bump diff
command to be able to apply a list of overlays to the files before
creating the diff.

The feature was asked by @JakeSCahill (thanks for asking it makes a
lot of sense to have it for the diff command too)

@paulRbr paulRbr self-assigned this Jul 6, 2025
Copy link
Member

@Polo2 Polo2 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, and the clear details about both refacto and new feature.

I understand we can call bump diff command with overlays, but I'd need more context to understand how to use this new feature.

Would it be possible to mention this new feature in output of bump diff --help, maybe in readme, and add some tests?

@fbraure fbraure requested a review from Polo2 July 15, 2025 21:41
@paulRbr paulRbr force-pushed the overlay-flag-for-diff branch 3 times, most recently from e825e0f to ee990f1 Compare July 16, 2025 14:13
This commit is both a refactoring (to move the logic to apply overlays
inside the `definition.extractDefinition` to DRY the code and simplify
extracting an overlayed definition) and a new feature.

The new feature is to allow the `--overlay` flag on the `bump diff`
command to be able to apply a list of overlays to the files before
creating the diff.

The feature was asked by @JakeSCahill (thanks for asking it makes a
lot of sense to have it for the diff command too)
@paulRbr paulRbr force-pushed the overlay-flag-for-diff branch from ee990f1 to 60bdb1a Compare July 16, 2025 14:16
@paulRbr
Copy link
Member Author

paulRbr commented Jul 16, 2025

friendly ping @Polo2

Copy link
Member

@Polo2 Polo2 left a comment

Choose a reason for hiding this comment

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

Many thanks for your patience, this new feature looks indeed a very good way to use overlay for diff purpose.

LGTM, thanks ✨

@paulRbr paulRbr merged commit f6d022c into bump-sh:main Jul 17, 2025
9 checks passed
@paulRbr paulRbr deleted the overlay-flag-for-diff branch July 17, 2025 07:30
paulRbr added a commit to paulRbr/cli that referenced this pull request Jul 22, 2025
This was overseen in bump-sh#710 where I applied overlays only on public
diffs, and not on authenticated diffs 🤦

Sorry about that!
paulRbr added a commit to paulRbr/cli that referenced this pull request Jul 22, 2025
This was overseen in bump-sh#710 where I applied overlays only on public
diffs, and not on authenticated diffs 🤦

Sorry about that!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants