Skip to content

Cleanup services module#1273

Merged
jridgewell merged 1 commit intoampproject:masterfrom
jridgewell:service-cleanup
Jan 5, 2016
Merged

Cleanup services module#1273
jridgewell merged 1 commit intoampproject:masterfrom
jridgewell:service-cleanup

Conversation

@jridgewell
Copy link
Copy Markdown
Contributor

  • Don’t call getServices with a second parameter (it’s not expecting it)
  • Reorder conditionals to make code clearer

- Don’t call `getServices` with a second parameter (it’s not expecting
it)
- Reorder conditionals to make code clearer
@ampsauce
Copy link
Copy Markdown

Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: 561d569
Build details: https://travis-ci.org/ampsauce/amphtml/builds/99502591

(Please note that this is a fully automated comment.)

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.

Web S=should probably have an assert here that s.obj is actually an object.

@cramforce
Copy link
Copy Markdown
Member

LGTM

@cramforce cramforce added the LGTM label Jan 1, 2016
@cramforce
Copy link
Copy Markdown
Member

(aside: Please add "NEEDS REVIEW" label when sending PR).

jridgewell added a commit that referenced this pull request Jan 5, 2016
@jridgewell jridgewell merged commit cf10b09 into ampproject:master Jan 5, 2016
@jridgewell jridgewell deleted the service-cleanup branch January 5, 2016 23:15
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.

3 participants