-
Notifications
You must be signed in to change notification settings - Fork 22
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
V03 wip issue575 ... adding "flow" component, and flowMain option. #588
Conversation
…ut containing second implementation of post, shovel, watch, and winnow components. Allows andre to use a stock component to host his amserver plugin.
allows all existing components to be built from pure configuration files. kind of next step in #575
I didn't merge this because I thought some discussion made sense... it is based on feedback from people about not liking or being confused initially by the concept of "components" ... so when I looked at what #379 that plugin from @andreleblanc11 cannot work with any existing component, so needed vanilla... question is... should we make everything vanilla, so there are no components any more? that would be future work... this just makes "vanilla" (aka: flow/ ) work Right now, flow/ is just added as an additional component... but it results in having two of everything... one implementation of each component as a sub-class, and another as a config file. Some of them are redundant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the changes. The explanations make sense.
To avoid the redundancy, for shovel, winnow, post and watch, we could just use flowMain shovel
, etc. right? Since we have to keep some subclasses, it doesn't seem terrible to keep the others even if they're just a storage location for the default config options
I didn't think adding three reviewers meant all three have to approve... oh well. we can discuss on Monday, and if bless it then. would be nice to have the additional reviews, but not the end of the world. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, love the docs changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
so this doesn't do the whole job of the issue, but I think it is useful on it's own, and it lets @andreleblanc11 's stuff work (he's using it in his branch also.)
it adds:
-- the ability to declare flow/ component configurations that are vanilla.
-- the ability to declare flowMain to run arbitrary subclasses of flow making it possible to define all existing components as config files.
-- include files to implement all the existing components.
-- English and French documentation of the above.