Skip to content

Remove snakecase package#102

Merged
sckott merged 3 commits intodevfrom
remove-snakecase
Feb 28, 2025
Merged

Remove snakecase package#102
sckott merged 3 commits intodevfrom
remove-snakecase

Conversation

@sckott
Copy link
Copy Markdown
Member

@sckott sckott commented Feb 28, 2025

fix #100

@sckott sckott requested a review from seankross February 28, 2025 17:51
@sckott sckott added this to the v0.2 milestone Feb 28, 2025
Copy link
Copy Markdown
Member

@seankross seankross left a comment

Choose a reason for hiding this comment

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

How do we expect this function to work? I am concerned because the functionality is not congruent:

> upper_camel_case("thisisatest")
[1] "Thisisatest"
> snakecase::to_upper_camel_case("thisisatest")
[1] "Thisisatest"
> upper_camel_case("this is a test")
[1] "This is a test"
> snakecase::to_upper_camel_case("this is a test")
[1] "ThisIsATest"
> upper_camel_case("this_is_a_test")
[1] "This_is_a_test"
> snakecase::to_upper_camel_case("this_is_a_test")
[1] "ThisIsATest"
> upper_camel_case("this-is-a-test")
[1] "This-is-a-test"
> snakecase::to_upper_camel_case("this-is-a-test")
[1] "ThisIsATest"
> upper_camel_case("this-is a_test")
[1] "This-is a_test"
> snakecase::to_upper_camel_case("this-is a_test")
[1] "ThisIsATest"

@github-actions
Copy link
Copy Markdown

@sckott
Copy link
Copy Markdown
Member Author

sckott commented Feb 28, 2025

Good catch.

So here's the bucket naming policy https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html

Having said that, this function is only used in the internal function bucket_to_policy_name, that's then used only in create_policy_if_missing, and in turn create_policy_if_missing is only used in two exported functions six_bucket_add_user and six_bucket_change_user.

And so we're only using the bucket name to create a policy name that includes the bucket name. So we're not trying to stick to some spec/standard/etc.

In commit I just pushed, changed the function and now

sixtyfour:::upper_camel_case("thisisatest")
#> [1] "Thisisatest"
snakecase::to_upper_camel_case("thisisatest")
#> [1] "Thisisatest"

sixtyfour:::upper_camel_case(string="this is a test")
#> [1] "Thisisatest"
snakecase::to_upper_camel_case("this is a test")
#> [1] "ThisIsATest"

sixtyfour:::upper_camel_case("this_is_a_test")
#> [1] "Thisisatest"
snakecase::to_upper_camel_case("this_is_a_test")
#> [1] "ThisIsATest"

sixtyfour:::upper_camel_case("this-is-a-test")
#> [1] "Thisisatest"
snakecase::to_upper_camel_case("this-is-a-test")
#> [1] "ThisIsATest"

sixtyfour:::upper_camel_case("this-is a_test")
#> [1] "Thisisatest"
snakecase::to_upper_camel_case("this-is a_test")
#> [1] "ThisIsATest"

sixtyfour:::upper_camel_case("this.is.a_test")
#> [1] "Thisisatest"
snakecase::to_upper_camel_case("this.is.a_test")
#> [1] "ThisIsATest"

Created on 2025-02-28 with reprex v2.1.1

So in my mind this is good as we just want a single string with all whitespace and spearators removed to go into the policy name.

Sound good?

@github-actions
Copy link
Copy Markdown

@seankross seankross self-requested a review February 28, 2025 19:58
Copy link
Copy Markdown
Member

@seankross seankross left a comment

Choose a reason for hiding this comment

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

Sounds good, thank you.

@sckott sckott merged commit c178940 into dev Feb 28, 2025
@sckott sckott deleted the remove-snakecase branch February 28, 2025 21:19
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.

2 participants