Skip to content

Refactor action invoke functions #319

Merged
houshengbo merged 7 commits intoapache:masterfrom
rabbah:feed
Jun 18, 2018
Merged

Refactor action invoke functions #319
houshengbo merged 7 commits intoapache:masterfrom
rabbah:feed

Conversation

@rabbah
Copy link
Copy Markdown
Member

@rabbah rabbah commented Jun 10, 2018

This permits a caller to determine how it wants to handle the action activation response. This is toward removing the feed output when creating a trigger with a feed.

The commits are all grouped and incremental for ease of reviewing.

This is related to fixing #134.

@rabbah
Copy link
Copy Markdown
Member Author

rabbah commented Jun 10, 2018

this is semantic preserving - and permits some further refactoring down the line to fix the referenced bug, which is part of #315 but is turning out to be fairly substantial so I am breaking things up into smaller pieces.

@rabbah rabbah force-pushed the feed branch 12 times, most recently from c506c29 to 24b1b52 Compare June 10, 2018 18:13
@rabbah
Copy link
Copy Markdown
Member Author

rabbah commented Jun 10, 2018

@csantanapr @dubee @houshengbo @mdeuser @pritidesai may I get one of you to look at this and approve a review 🙏 ?

Comment thread commands/action.go
parameters interface{},
blocking bool,
result bool) (map[string]interface{}, error) {
// TODO remove all global modifiers
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which global variables?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention is to remove all Client. and Flags. assignments where for the latter beyond the initial parsing should be viewed as immutable.

Copy link
Copy Markdown

@houshengbo houshengbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@houshengbo houshengbo merged commit 72bc486 into apache:master Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants