-
Notifications
You must be signed in to change notification settings - Fork 41
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
exhaust: added exhaust function, with stub for exhaust-csv-export and… #441
exhaust: added exhaust function, with stub for exhaust-csv-export and… #441
Conversation
… a sample impl Signed-off-by: Paz Barda <[email protected]>
737e873
to
a9af7dd
Compare
…so need to imnplement configuration of: file name, custom headers (optional) Signed-off-by: Paz Barda <[email protected]>
a9af7dd
to
c6b8484
Compare
Signed-off-by: Paz Barda <[email protected]>
e1583a5
to
461f4f7
Compare
Signed-off-by: Paz Barda <[email protected]>
src/lib/exhaust.ts
Outdated
import {ExhaustExportCsv} from '../models/exhaust-export-csv'; | ||
|
||
const createExhaustPlugin = (pluginTypeName: string) => { | ||
switch (pluginTypeName) { |
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.
we should discuss if plugins are another repository or builtins
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 think we got approval to import exhaust plugins as we are doing with ordinary plugins
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.
in retrospect i think this makes sense.
looking at the way ordinary plugins are being created from the manifest, i think applying a similar flow for creation of the exhaust plugins means significant changes relative to the PR state as it is right now.
i still think we should do it :) but maybe not in the scope of this task.
i propose maybe pushing the csv-export exhaust functionality as it is in this PR, and create a new issue (with the relevant priority) for refactoring it into external plugins. this actually means 2 PRs: one for IF exhaust infra and the other for a CSV exporter exhaust plugin.
The main reason im proposing it is that Joseph said in last weekly call he is very eager to be able to show the csv-export functionality.
WDYT?
e0d36c5
to
7db9860
Compare
e05231f
to
171a812
Compare
… a sample impl Signed-off-by: Paz Barda <[email protected]>
…so need to imnplement configuration of: file name, custom headers (optional) Signed-off-by: Paz Barda <[email protected]>
Signed-off-by: Paz Barda <[email protected]>
Signed-off-by: Paz Barda <[email protected]>
Signed-off-by: Paz Barda <[email protected]>
…so added id column to csv Signed-off-by: Paz Barda <[email protected]>
…tead of piping them Signed-off-by: Paz Barda <[email protected]>
Signed-off-by: Paz Barda <[email protected]>
171a812
to
fcbf885
Compare
Signed-off-by: Paz Barda <[email protected]>
… a sample impl
Types of changes
A description of the changes proposed in the Pull Request