Skip to content

Conversation

paulRbr
Copy link
Member

@paulRbr paulRbr commented Dec 19, 2024

This is a fix after the major upgrade of the CLI (in #587) where we
broke the interface of the exported core Diff class and made its
usage as a lib more complicated.

Let's align back to what it used to be, and even simplify the
interface by making the constructor argument optional.

This is a fix after the major upgrade of the CLI (in bump-sh#587) where we
broke the interface of the exported core `Diff` class and made its
usage as a lib more complicated.

Let's align back to what it used to be, and even simplify the
interface by making the constructor argument optional.
@paulRbr paulRbr self-assigned this Dec 19, 2024
@paulRbr paulRbr requested a review from fbraure December 19, 2024 19:32
@paulRbr
Copy link
Member Author

paulRbr commented Dec 19, 2024

@fbraure this is the PR I talked to you about yesterday. I broke the interface of the exported Diff lib in #587 so this PR fixes the signature, and it also allows to pass no arguments with a default behavior on the config.

Could you please review so I can release a new version of the CLI, and then be able to finish bump-sh/github-action#508 please 🙏

@paulRbr paulRbr force-pushed the fix-core-diff-interface branch from f965582 to 60d0542 Compare December 19, 2024 19:40
Copy link
Contributor

@fbraure fbraure left a comment

Choose a reason for hiding this comment

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

Overall, it looks good to me. It's just the configuration path that's a mystery to me.

Copy link
Contributor

@fbraure fbraure left a comment

Choose a reason for hiding this comment

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

LGTM

@paulRbr paulRbr merged commit ed870f7 into bump-sh:main Dec 19, 2024
9 checks passed
@paulRbr paulRbr deleted the fix-core-diff-interface branch January 13, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants