Skip to content

Improves loading of Push Adapter, fix loading of S3Adapter#833

Merged
flovilmart merged 2 commits intomasterfrom
flovilmart.S3Environment
Mar 6, 2016
Merged

Improves loading of Push Adapter, fix loading of S3Adapter#833
flovilmart merged 2 commits intomasterfrom
flovilmart.S3Environment

Conversation

@flovilmart
Copy link
Contributor

@flovilmart flovilmart commented Mar 4, 2016

  • Adds environment variables to configure S3Adapter

Fixes #820
Fixes #832

@drew-gross
Copy link
Contributor

drew-gross commented Mar 4, 2016

Can you also update the docs to describe the new options?

@flovilmart
Copy link
Contributor Author

flovilmart commented Mar 4, 2016

Sure!

@facebook-github-bot
Copy link

facebook-github-bot commented Mar 4, 2016

@flovilmart updated the pull request.

@drew-gross drew-gross closed this Mar 4, 2016
@drew-gross drew-gross reopened this Mar 4, 2016
@codecov-io
Copy link

codecov-io commented Mar 4, 2016

Current coverage is 91.66%

Merging #833 into master will decrease coverage by -0.02% as of 80f99d5

@@            master    #833   diff @@
======================================
  Files           71      71       
  Stmts         4100    4103     +3
  Branches       843     845     +2
  Methods          0       0       
======================================
+ Hit           3759    3761     +2
  Partial         10      10       
- Missed         331     332     +1

Review entire Coverage Diff as of 80f99d5

Powered by Codecov. Updated on successful CI builds.

@flovilmart flovilmart force-pushed the flovilmart.S3Environment branch from 2fbcd1b to 764c563 Compare March 4, 2016 18:36
@facebook-github-bot
Copy link

facebook-github-bot commented Mar 4, 2016

@flovilmart updated the pull request.

asciimike
asciimike reviewed Mar 4, 2016
View reviewed changes
src/Adapters/AdapterLoader.js Outdated
return loadAdapter(defaultAdapter, undefined, adapter);
} catch (e) {};
}
// return the adapter as is as it's unusable otherwise
Copy link
Contributor

@asciimike asciimike Mar 4, 2016

Choose a reason for hiding this comment

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

Is this comment still correct, or will it just return the object representing the adapter (which is presumably usable)?

Copy link
Contributor Author

@flovilmart flovilmart Mar 4, 2016

Choose a reason for hiding this comment

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

Yeah? That retuns the adapter as not other loading method seems to be available.

Copy link
Contributor

@asciimike asciimike Mar 4, 2016

Choose a reason for hiding this comment

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

Oh man, I totally misread that, somehow turned that into "return the adapter as it's unusable". Might I suggest "return the adapter as provided" or something similar?

Copy link
Contributor Author

@flovilmart flovilmart Mar 4, 2016

Choose a reason for hiding this comment

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

👍

@asciimike asciimike mentioned this pull request Mar 4, 2016
Merged
@flovilmart flovilmart force-pushed the flovilmart.S3Environment branch from 764c563 to a44b1d9 Compare March 4, 2016 23:09
@facebook-github-bot
Copy link

facebook-github-bot commented Mar 4, 2016

@flovilmart updated the pull request.

flovilmart added a commit that referenced this pull request Mar 6, 2016
@flovilmart
Merge pull request #833 from ParsePlatform/flovilmart.S3Environment
375e701 
Improves loading of Push Adapter, fix loading of S3Adapter
@flovilmart flovilmart merged commit 375e701 into master Mar 6, 2016
@flovilmart flovilmart deleted the flovilmart.S3Environment branch March 6, 2016 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@flovilmart flovilmart

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@flovilmart @drew-gross @facebook-github-bot @codecov-io @asciimike @gfosco