Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #12] Introduce feature flags to make extension of io/Coercions optional #11

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

vemv
Copy link
Contributor

@vemv vemv commented Dec 12, 2021

These allow dependent libraries to use fs without the side-effect of extend-protocol.

Otherwise one can unawarely create issues, as seen in https://github.com/clojure-emacs/clj-refactor.el/issues/508.

https://clojure.org/reference/protocols#_guidelines_for_extension says:

If you don’t own the protocol or the target type, you should only extend in app (not public lib) code, and expect to maybe be broken by either owner.

So what fs was doing wasn't ideal anyway.

The proposed feature flag is the best way I found such that:

  • the existing behavior is preserved
  • refactor-nrepl can effectively require this ns and modify the feature flag without affecting anyone else
  • Regular apps can also change the value if desired
    • it's a little more risky for them because they don't have mranderson. But OTOH an app is more of "closed system" which can be quite exhaustively unit-tested.

Cheers - V

These allow dependent libraries to use `fs` without the side-effect of extend.

Otherwise one can unawarely create issues, as seen in https://github.com/clojure-emacs/clj-refactor.el/issues/508.

https://clojure.org/reference/protocols#_guidelines_for_extension says:

> If you don’t own the protocol or the target type, you should only extend in app (not public lib) code, and expect to maybe be broken by either owner.
@slipset
Copy link
Member

slipset commented Dec 14, 2021

I think this is a valid approach, and seems to solve the downstream problem.

@slipset slipset changed the title Introduce feature flags [Fix #12] Introduce feature flags to make extension of io/Coercions optional Dec 14, 2021
@slipset slipset merged commit 6002681 into clj-commons:master Dec 14, 2021
@vemv vemv deleted the feature-flag branch December 14, 2021 08:27
@vemv
Copy link
Contributor Author

vemv commented Dec 14, 2021

Cheers, worked nicely clojure-emacs/refactor-nrepl#356

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.

None yet

2 participants