Feature: add s3 key name prefixes #202
Conversation
ericgriffis
changed the title
.travis.yml
Outdated
| rvm: | ||
| - 2.0.0 |
There was a problem hiding this comment.
Why did you drop support for older versions of Ruby? This is as far as I can tell unrelated to this PR?
lib/capybara-screenshot/s3_saver.rb
Outdated
| ) | ||
|
|
||
| s3_client = Aws::S3::Client.new(s3_client_credentials) | ||
| bucket_name = configuration.fetch(:bucket_name) | ||
| bucket_name = configuration.delete(:bucket_name) |
There was a problem hiding this comment.
Why delete as opposed to fetch. This will now mutate configuration which is a side effect of a class method. I don't understand why you would do this?
lib/capybara-screenshot/s3_saver.rb
Outdated
| @@ -5,10 +5,11 @@ module Screenshot | |||
| class S3Saver | |||
| DEFAULT_REGION = 'us-east-1' | |||
|
|
|||
| def initialize(saver, s3_client, bucket_name) | |||
| def initialize(saver, s3_client, bucket_name, key_prefix: nil) | |||
There was a problem hiding this comment.
This syntax is not compatible with older versions of Ruby. If I recall, we are not using named args like this anywhere else, so would prefer for not to remain consistent by using options = {}
lib/capybara-screenshot/s3_saver.rb
Outdated
|
|
||
| new(saver, s3_client, bucket_name) | ||
| new(saver, s3_client, bucket_name, **configuration) |
There was a problem hiding this comment.
If you change to options for consistency, then you just pass in configuration?
ericgriffis
force-pushed
the
feature/add_s3_key_name_prefixes
branch
from
d815085 to
6d94922
Compare
|
@mattheworiordan Made the requested changes and rebased to master... Let me know if there's any other issues |
|
Excellent, thank you very much for this contribution! |
|
This has now been released in |
Allows configuration of a key prefix for S3 uploads. This allows custom folders for uploads within your bucket, fulfilling #194.
Example of new configuration: