Skip to content

v6 updates - #210

Merged
smaeda-ks merged 15 commits into
xdevplatform:masterfrom
smaeda-ks:smaeda-ks/develop/v6
Sep 18, 2019
Merged

v6 updates#210
smaeda-ks merged 15 commits into
xdevplatform:masterfrom
smaeda-ks:smaeda-ks/develop/v6

Conversation

@smaeda-ks

@smaeda-ks smaeda-ks commented Sep 15, 2019

Copy link
Copy Markdown
Contributor
@smaeda-ks smaeda-ks added the task label Sep 15, 2019
@smaeda-ks smaeda-ks self-assigned this Sep 15, 2019
# set default values
job_ids = opts.fetch(:job_ids, nil)
params = {}
params[:job_ids] = Array.wrap(job_ids).join(',') if job_ids

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

bug: Array#wrap is only available for ActiveSupport in Rails. This was introduced in b1f2e97#diff-697bfc02b2bb3faa338ddbb781e0fb93 (2017)

@smaeda-ks smaeda-ks changed the title [WIP] v6 updates Sep 15, 2019

@tushdante tushdante left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor changes, but otherwise 👍

Comment thread lib/twitter-ads/account.rb
Comment thread lib/twitter-ads/resources/analytics.rb Outdated
Comment thread lib/twitter-ads/resources/analytics.rb Outdated
@smaeda-ks
smaeda-ks requested a review from tushdante September 18, 2019 03:02

@tushdante tushdante left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall LGTM! Just wanted to get your thoughts on the active entities question

Comment thread lib/twitter-ads/resources/analytics.rb
@smaeda-ks
smaeda-ks merged commit d7a99e3 into xdevplatform:master Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants