search

collectiveidea / interactor

Interactor provides a common interface for performing complex user interactions.
MIT License
3.36k stars 212 forks source link

Please add upgrade guide for v3 and improve README #61

Closed PikachuEXE closed 7 years ago

PikachuEXE commented 10 years ago

Some changes are not well explained like:

Also run! is not mentioned in README

Thank you for your hard work, I like interactor very much :)

TODO

laserlemon commented 10 years ago

An upgrade guide is a good idea, thank you! I'll make clearer mention of the halting behavior of fail!as well.

The change in behavior regarding "magical" context access is a better topic for the upgrade guide.

As for the run! method, it isn't meant to be part of the public API. Interactors should be invoked using the call class method, which I believe is explained in the readme. One of the next tasks I'd like to tackle is to comment the code using TomDoc, which should help clarify the line between public and private APIs.

Thank you for your feedback and please continue to report any issues you find with version 3!

laserlemon commented 10 years ago

I've started the tomdoc branch for adding code documentation (in progress).

laserlemon commented 10 years ago

Milestone cleared as the only enhancement important enough to warrant a patch release would be code documentation. Opened pull request #63 under the 3.0.1 milestone.

seako commented 10 years ago

While upgrading to v3, I was a little surprised to find that Interactor's initializer is considered part of the internal api and also that Interactor.call returns an instance of Interactor::Context rather than Interactor. In fact, there doesn't appear to be a way (that's part of the documented public api) to get an instance of an Interactor anymore. Perhaps you could provide some insight into these implementation choices?

laserlemon commented 10 years ago

The public interface to an Interactor class is limited to Interactor.call. We discuss two issues specifically related to your observations in issues #47 and #49.

seako commented 10 years ago

thank you very much!

laserlemon commented 10 years ago

Instances of interactors are now throwaways as the context has (or should have) all of the information resulting from invocation of the interactor.

laserlemon commented 10 years ago

You're very welcome!

beNjiox commented 10 years ago

hey @laserlemon, As I told you in https://github.com/collectiveidea/interactor/issues/64#issuecomment-56539591 I've started working on a wiki on a fork. https://github.com/beNjiox/interactor/wiki.

I was also thinking of covering the upgrade guide as well as the halting behavior of fail!, if it's ok for you. Feel free to let me know your thoughts.

laserlemon commented 10 years ago

That's great, thank you! Please continue as you're able. The progress you're making is super helpful! :clap:

beNjiox commented 10 years ago

Hey @laserlemon,

I've been working a little bit on my fork version.

I've done some little changes to the README so that it mentions halting behavior of fail! as well as referencing the new wiki when mentionning multiple hooks.

I've also lightened the Upgrade Guide of the wiki to something simple, I realized that It doesn't need to be heavy since upgrading is not that difficult.

To conclude, I've created a simple app using interactor for people who wants to see it in action. I think it can help people to jump in and see the power of it. I'm curious to know what you think about it!

Let me know if you want some more changes, and if you want to integrate my fork to the main repo.

Thanks,

laserlemon commented 7 years ago

Closing because… stale. ☹️