Open todd opened 5 years ago
Thank you for your work and the opportunity to comment, however we had to move on to something else for our workflow solution. In general, the coding on this project is some of the most impressive and advanced coding I've seen, especially for JS / TS. But this also makes it very challenging to understand what the package does just by reviewing the code, and the documentation is very sparse, so I just didn't have confidence we were going to be able to make it work. Also, the dearth of activity, contributors, and releases was concerning. IMHO, to be successful, the documentation can't just be some examples. I never could figure out what the core operations of the package were, how they interacted, all the built in behavior, the primary algorithms (conceptually), etc.
Very impressive piece of work, but I think you all would get a lot more traction with it if you could work up a good, comprehensive white paper on how it works at a higher level, the key components, how they interact, how to implement a workflow with it, etc. It was the most promising package I found after lots of searching for a JS based workflow module that was free open source and very extensible.
Thanks again for your work, and good luck with the package.
John K.
On Tue, Jun 18, 2019 at 2:23 AM Todd Bealmear notifications@github.com wrote:
Re: #158 https://github.com/ifandelse/machina.js/issues/158
Almost a year after I submitted the initial issue and six months after I took my first stab at this, I've finally got something somewhat useable (big yikes, sorry for the delay). This isn't ready for a merge just yet, but I wanted to get this open to solicit feedback.
Pinging @ifandelse https://github.com/ifandelse, @miedmondson https://github.com/miedmondson, and @king612 https://github.com/king612 for their thoughts since they were the participants in the original thread.
A couple of notes:
- This only covers BehavioralFsm for the moment as that's what I use primarily and it's the more complicated of the two classes. Should we be happy with this implementation, it should be easy enough to abstract and apply to Fsm.
- I fear I may have gone a bit overboard with the type safety here in order to make things as inspectable as possible. I'm particularly interested in feedback on how usable folks think this implementation is - you can see how to use this in types/BehavioralFsmTests.ts, which implements the traffic light example from the docs.
- If you've ever used TypeScript with React, some of the example implementation may look familiar to you. My implementation was heavily inspired by those typings.
- An aside: the extensible nature of Machina through an options constructor made writing these fairly complicated and a bit brittle. That's not a criticism of the underlying library, just a fact that I wrestled with when putting this together. An abstraction or refactor of these to utilize ES6 classes could aid in the maintainability of these types. Not recommending we go this route (certainly not immediately, anyway), just making an observation.
Should we like where this is headed, I can clean this up a bit more, add the types and tests for Fsm, and plug the transpiler into the build pipeline so we ensure clean builds as part of CI. Additionally, anyone who would like write access to my fork to help work on these should feel free to reach out.
Let me know what you think, team.
You can view, comment on, or merge this pull request online at:
https://github.com/ifandelse/machina.js/pull/165 Commit Summary
- :construction: First Pass
File Changes
- M .gitignore https://github.com/ifandelse/machina.js/pull/165/files#diff-0 (1)
- M package.json https://github.com/ifandelse/machina.js/pull/165/files#diff-1 (7)
- A tsconfig.json https://github.com/ifandelse/machina.js/pull/165/files#diff-2 (63)
- A types/BehaviorialFsmTests.ts https://github.com/ifandelse/machina.js/pull/165/files#diff-3 (118)
- A types/index.d.ts https://github.com/ifandelse/machina.js/pull/165/files#diff-4 (61)
Patch Links:
- https://github.com/ifandelse/machina.js/pull/165.patch
- https://github.com/ifandelse/machina.js/pull/165.diff
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ifandelse/machina.js/pull/165?email_source=notifications&email_token=ABLDDUD23NOAO6RBWGS4OJLP3B5PFA5CNFSM4HY4WDC2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G2BS7UA, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLDDUCRB5CLMBN5GKMN7CLP3B5PFANCNFSM4HY4WDCQ .
Re: #158
Almost a year after I submitted the initial issue and six months after I took my first stab at this, I've finally got something somewhat useable (big yikes, sorry for the delay). This isn't ready for a merge just yet, but I wanted to get this open to solicit feedback.
Pinging @ifandelse, @miedmondson, and @king612 for their thoughts since they were the participants in the original thread.
A couple of notes:
BehavioralFsm
for the moment as that's what I use primarily and it's the more complicated of the two classes. Should we be happy with this implementation, it should be easy enough to abstract and apply toFsm
.types/BehavioralFsmTests.ts
, which implements the traffic light example from the docs.Should we like where this is headed, I can clean this up a bit more, add the types and tests for
Fsm
, and plug the transpiler into the build pipeline so we ensure clean builds as part of CI. Additionally, anyone who would like write access to my fork to help work on these should feel free to reach out.Let me know what you think, team.