Existing Feature

Proposed Problem (1)

Effortless spec-based stub generation in nil-body functions for rapid prototyping is a potentially useful tool
Currently this is enabled by default, which is a jarring experience if one is not aware of this feature; as explained in https://github.com/gnl/ghostwheel/issues/26#issuecomment-496353977

Proposed Solution (1)

Improve documentation: the feature is mentioned in the readme #introduction, but that would also be a good place to mention (or link to) how to disable/enable it.
The feature is not mentioned :ghostwheel.core/ghostwheel-config nor ghostwheel.utils/ghostwheel-default-config
Honestly, as a user - I am still not sure if and how I can disable this feature.
It's a jarring experience if one is not aware of it, so make it opt-in via configuration.

Alternative Feature

Once I started thinking about this problem, I'd like to suggest an alternative approach.

Proposed Problem (2)

Low-effort spec-based stub generation of functions for rapid prototyping is a potentially useful tool
The principle of least-surprise is important to Clojure developers
Configuration is global on/off, while syntax can be applied locally
The road from "cool dev tool" to "potential bug in production" is paved with good intentions (i.e. how easy is it for dev to accidentally not implement a function, but have it "work" during dev/testing?)

Proposed Solution (2)

Instead of global enabled/disabled feature of nil-body functions, we could configure it via metadata (similar to ::g/trace). That way:

requires no configuration
is opt-in for any function
concise enough to easily type (small overhead vs an implicit global configuration)
explicit in what is not yet implemented (easy to grep in codebase, etc.)

(>defn ^::g/fake some-string-to-int
  [x] [string? => int?]
  nil)

We could even take this even a step-further and remove the nil-body constraint. Any time we add the metadata key ::g/fake to a function, it will simply ignore the body and generate random data from spec. Possible use-cases for this? Stubbing out in development an I/O function, etc.

What we call the metadata key itself (::g/fake) is open to discussion. I'm just wondering if this idea has any legs. Thoughts?

gnl commented 5 years ago

This is definitely a problem and yes, at the moment it cannot be turned off. There is already a fix in the in-progress 0.4.0 release, which is that nil-body functions work precisely as you would expect them to, but no-body functions generate the stub and print a warning in the console about the fact when called.

I am considering adding an option to disable it, which would work like all the others with overrideable metadata and all that. Might also be better to still default to off, because its usage doesn't seem that common to justify having it on by default.

What I'm also thinking about is overriding the defn specs in clojure.core.specs.alpha to disallow body-less defns, so that if you forgot a stub somewhere your production build wouldn't even compile. At the moment it's perfectly legit to do (defn foo [a b]) in Clojure, which doesn't make much sense to me.

The force-stubbing a function with an existing body is an interesting idea, but I'm not quite sure about it. If you enable the option with metadata during development and everything seems to work so that you forget about it, in production it would of course not be stubbed and things might break without you having properly tested it in development.

Seems like an easy mistake to make and I'd rather force the programmer to comment out the function body if they want something stubbed, because this seems harder to screw up, especially in combination with the potential disallowing of body-less functions mentioned above.

gnl commented 4 years ago

👋 Hi. I'm posting this same comment to all issue threads to just give a quick heads up that the project, despite rumours and some evidence to the contrary, is not dead. It was hibernating for a little while and now nearing the long-awaited next release, which will fix some long-standing issues (and introduce some breaking changes to the config).

I'll be reviewing all open issues and PRs over the next couple of weeks, so stay tuned and thanks for the patience.

gnl / ghostwheel