8000 Add method and URL to Event, add method to EventDispatcher by delikat · Pull Request #1 · optimizely/ruby-sdk · GitHub
[go: up one dir, main page]
Skip to content

Add method and URL to Event, add method to EventDispatcher #1

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Aug 29, 2016
Merged

Add method and URL to Event, add method to EventDispatcher #1

merged 7 commits into from
Aug 29, 2016

Conversation

delikat
Copy link
Contributor
@delikat delikat commented Aug 25, 2016 •
edited
Loading
  • Adds method and URL properties to Event
  • Adds method argument to EventDispatcher.dispatch_event
  • Renames EventBuilder to EventBuilderV1
  • Removes abstract BaseEventBuilder class (https://github.com/bbatsov/ruby-style-guide#duck-typing)
  • Locks down a few unnecessary attr_accessors to attr_readers
  • Fixes broken tests

@aliabbasrizvi @delikat @haleybash-optimizely @mikeng13 @alda-optimizely @wangjoshuah @jophde @jsm

@delikat delikat assigned delikat and unassigned delikat Aug 26, 2016
mikeproeng37
mikeproeng37 reviewed Aug 26, 2016
View reviewed changes
lib/optimizely/event_builder.rb

# Gets/Sets event params.
attr_accessor :params
attr_reader :method
Copy link
Contributor
@mikeproeng37 mikeproeng37 Aug 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

these all need docs. Method is not a very descriptive name. A developer seeing this might be tempted to pass in a function. Maybe method type or http verb/action is more suitable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I like http_verb.

@aliabbasrizvi
Copy link
Contributor

LGTM

@delikat delikat merged commit 50fe140 into v2_support Aug 29, 2016
@delikat de 7636 likat deleted the delikat/event_dispatcher_method branch August 29, 2016 19:06
msohailhussain pushed a commit that referenced this pull request Dec 3, 2019
@jasonkarns
Merge pull request #1 from optimizely/callable-fix-lint-errors
f11cf80 
fix: lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@delikat @aliabbasrizvi @mikeproeng37
0