Skip to content

Overriding + changes semantics; plays badly with linters #584

@duncan-bayne

Description

@duncan-bayne

Hi! Thanks for minimagick, we've been happily using it at work for years.

I recently broke some of my code by running rubocop --autocorrect-all on it. The source of the breakage was a seemingly innocuous removal of a no-op +; it rewrote:

commands.sigmoidal_contrast + '11.5'

... to ...

commands.sigmoidal_contrast

This broke a bunch of tests; upon investigation it turns out that minimagick overrides + to change the state of the MiniMagick::Tool class.

Looking at the history, it seems this was done for ergonomic reasons, but I think it falls foul of the principle of least astonishment. I was certainly astonished 😉

Obviously, though, simply changing the API would itself be a breaking change. Might it be possible to deprecate this override for a release, then entirely kill it off?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions