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

Config flatten #639

Merged
merged 22 commits into from
Jul 4, 2023
Merged

Config flatten #639

merged 22 commits into from
Jul 4, 2023

Conversation

DyfanJones
Copy link
Member

@DyfanJones DyfanJones commented Jun 28, 2023

Initial idea for flattening the config list for initialising services (ticket: #421)

proposal 2

svc(config = list(), ...)
# current config list
s3  <- paws::s3(config = list(credentials = list(profile = "dummy"), region = "us-east-1"))

# flatten config short hand
s3  <- paws::s3(profile = "dummy", region = "us-east-1")

# flatten config short hand and config list together
s3  <- paws::s3(config = list(region = "us-east-1"), profile = "dummy")

Downside:
Doesn't help with auto-complete as the short hand utilises ... instead of individual parameters.

Upside:
Allows the short hand method to keep up to date with any parameters that get added to the paws.common backend.

@DyfanJones
Copy link
Member Author

@davidkretch What's your view on this? I am just toying around with this idea so not tied to it as of yet.

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.23 ⚠️

Comparison is base (71c6594) 83.88% compared to head (fe8384c) 83.66%.

❗ Current head fe8384c differs from pull request most recent head c3fb7d8. Consider uploading reports for the commit c3fb7d8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #639      +/-   ##
==========================================
- Coverage   83.88%   83.66%   -0.23%     
==========================================
  Files         189      195       +6     
  Lines       12934    13213     +279     
==========================================
+ Hits        10850    11054     +204     
- Misses       2084     2159      +75     

see 21 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@DyfanJones
Copy link
Member Author

After discussion in #421 will go for proposal 1. As it helps with auto-complete when initialising service.

svc <- acm(
  config = list(),
  credentials = list(
    creds = list(
      access_key_id = "string",
      secret_access_key = "string",
      session_token = "string"
    ),
    profile = "string"
  ),
  endpoint = "string",
  region = "string"
)

@DyfanJones DyfanJones marked this pull request as ready for review July 4, 2023 16:25
@DyfanJones DyfanJones merged commit 69e98ed into paws-r:main Jul 4, 2023
@DyfanJones DyfanJones deleted the config_flattern branch September 11, 2023 11:23
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.

1 participant