github / github/scientist

[Feature request] Allow `Scientist::Experiment` classes to *not* be default Scientist experiment class

Aperta
#162 15 commenti 7 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Ruby
Stelle
7.8k
Fork
505
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

I'm wondering what the original inspiration for making any class that `include`s the module the default experiment, [code here](https://github.com/github/scientist/blob/303c1f4ff98786b702507ed0c4f8cfdeb5fc7138/lib/scientist/experiment.rb#L13). Is it possible to create a class that `include`s the `Scientist::Experiment` module and **not** make it the default scientist experiment?

Looking [here](https://github.com/github/scientist#making-science-useful) it doesn't seem like it is possible.

From docs as well:
> When `Scientist::Experiment` is included in a class, it automatically sets it as the default implementation via `Scientist::Experiment.set_default`. This `set_default` call is is skipped if you include `Scientist::Experiment` in a module.

The reason I ask is because recently we ran into an issue with our Rails app:
1. We had a class that was running `Scientist` via `include` `Scientist` with the default experiment, but was **not** publishing anything, *oops*.
2. We created a new class for that `include`d `Scientist::Experiment`, but since our tests don't eager load our classes, it was a flaky test and not surfaced during our build.
3. When running in production, we eager loaded and overrode the default experiment and then surfaced issues.

So a few questions I was wondering:
1. Why default experiment, this seems a little aggressive IMO?
2. Can we actually find a way to either make default the normal behavior and have a way to pass a flag to override it?
3. Or is it that the way we were using it was wrong, and that the intention is to actually just `include Scientist` and override the default methods for that class? If so, the only reason I held back from doing so is better separation of responsibilities. It was going to look a bit messy to clog up one class's specs with experiment specs and I'd rather separate the two easily 🤔
4. Relating to point 3, is it the case that by the current implementation we can't be running multiple experiments at once?

I looked into it a bit myself, but it doesn't seem like there is a reasonably clean way. Although I think we can add a class method (something like):

```
class Foo
include Scientist::Experiment

default_scientist_experiment(false) # new
end
```

What do y'all think? If so - I can try to take a stab at it.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.