Skip to content
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.

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

Remove AWS key and secret from configuration #49

Closed
wants to merge 1 commit into from

Conversation

acinader
Copy link
Contributor

@acinader acinader commented Jul 1, 2017

Instead let the aws sdk take care of credentials.

fixes: #20

Instead let the aws sdk take care of credentials.

fixes: 20
@acinader acinader requested a review from flovilmart July 1, 2017 22:13
@codecov
Copy link

codecov bot commented Jul 1, 2017

Codecov Report

Merging #49 into master will decrease coverage by 30.49%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
- Coverage   93.98%   63.49%   -30.5%     
==========================================
  Files           2        2              
  Lines         133      126       -7     
  Branches       27       26       -1     
==========================================
- Hits          125       80      -45     
- Misses          8       46      +38
Impacted Files Coverage Δ
lib/optionsFromArguments.js 87.27% <ø> (-9.34%) ⬇️
index.js 45.07% <ø> (-46.83%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cedb8e...b3104e6. Read the comment docs.

Copy link
Contributor

@flovilmart flovilmart left a comment

Choose a reason for hiding this comment

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

Should we put all options into a single object instead of having 2 arguments?

@acinader
Copy link
Contributor Author

acinader commented Jul 8, 2017

I think the most typical case will be configuration using the single string for the bucket name. But, let me re-look at it with your suggestion and also considering simplifying the code/better option/env handeling. I think that this can certainly be simplified. I don't think we want to publish this for a couple of weeks and I'm traveling, so may be a bit till I get to it...but I will.

@davimacedo
Copy link
Member

Closing for being stale.

@davimacedo davimacedo closed this Oct 8, 2020
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.

Don't accept Key and Secret as constructor arguments
3 participants