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

AWS API Definitions Updated #637

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Conversation

github-actions[bot]
Copy link
Contributor

Automated changes by create-pull-request GitHub action

@github-actions github-actions bot force-pushed the create-pull-request/patch branch 2 times, most recently from fd68721 to 3266ffc Compare June 20, 2023 06:15
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 5 times, most recently from 032cc9f to 595dcd3 Compare June 27, 2023 06:15
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 5 times, most recently from b9192b9 to 3cbba8a Compare July 4, 2023 06:16
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 3 times, most recently from e26bbbb to 679ba6a Compare July 8, 2023 06:15
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 5 times, most recently from 1e9fdd6 to b613dcf Compare July 21, 2023 06:14
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 6 times, most recently from 336477c to a52cd6b Compare July 29, 2023 06:12
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 5 times, most recently from 4ddb83a to 18143d8 Compare June 8, 2024 06:13
@mortenpi
Copy link
Contributor

mortenpi commented Jun 11, 2024

Could this be merged and tagged? I noticed that AWS.jl doesn't have all the newest actions anymore (like SESv2 GetMessageInsights).

@ararslan
Copy link
Member

There's a lot of history that's collapsed by default but of note are #637 (comment) and #637 (comment). I agree it'd be nice to have it merged.

@mortenpi
Copy link
Contributor

Ah, yea, I did not check if there were any comments there.

For the formatting, I suggest we downgrade JuliaFormatter for the time being (#676). I think that should reduce the diff here.

It does look like the new formatting of those function definitions better matches the generated code, so it feels like the new JuliaFormatter is doing the right thing here.

if required_keys && (idempotent || headers)
return """
$formatted_function_name($(join(req_keys, ", ")); aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", $params_headers_str; aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
$formatted_function_name($(join(req_keys, ", ")), params::AbstractDict{String}; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", Dict{String, Any}(mergewith(_merge, $params_headers_str, params)); aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
"""
elseif !required_keys && (idempotent || headers)
return """
$formatted_function_name(; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", $params_headers_str; aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
$formatted_function_name(params::AbstractDict{String}; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", Dict{String, Any}(mergewith(_merge, $params_headers_str, params)); aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
"""
elseif required_keys && !isempty(req_kv)
return """
$formatted_function_name($(join(req_keys, ", ")); aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", $req_str); aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
$formatted_function_name($(join(req_keys, ", ")), params::AbstractDict{String}; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", Dict{String, Any}(mergewith(_merge, $req_str), params)); aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
"""
elseif required_keys
return """
$formatted_function_name($(join(req_keys, ", ")); aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\"; aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
$formatted_function_name($(join(req_keys, ", ")), params::AbstractDict{String}; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", params; aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
"""
else
return """
$formatted_function_name(; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\"; aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
$formatted_function_name(params::AbstractDict{String}; aws_config::AbstractAWSConfig=global_aws_config()) = $service_name(\"$method\", \"$request_uri\", params; aws_config=aws_config, feature_set=SERVICE_FEATURE_SET)
"""
end
end

But I think we could get the substantial changes here merged first, and then just do a trivial formatting PR afterwards.

@ararslan
Copy link
Member

It does look like the new formatting of those function definitions better matches the generated code, so it feels like the new JuliaFormatter is doing the right thing here.

Yeah I suppose it does better match the way the generated code is written, but the change seems like a bug to me; IIUC, BlueStyle is supposed to have short_to_long_function_def = true by default, which should have turned any short-form definition into a long-form (i.e. with a function block) definition when the line length exceeds 92-characters.

@ararslan
Copy link
Member

ararslan commented Jun 12, 2024

The diff here has been reduced very significantly thanks to @mortenpi; the remaining changes should now be reflective of actual AWS changes. @mattBrzezinski, given that the SQS change from QueryService to JSONService is expected, is there anything barring merging this? I suppose there's likely been further changes pushed to this PR since you last looked, do you think you'd have time to give it another look, or is there anything others can do to support that?

@github-actions github-actions bot force-pushed the create-pull-request/patch branch 5 times, most recently from 2ad97a6 to 817f639 Compare June 19, 2024 06:14
@github-actions github-actions bot force-pushed the create-pull-request/patch branch 3 times, most recently from b5c29a8 to 1a5f283 Compare June 25, 2024 06:15
@mattBrzezinski
Copy link
Member

The diff here has been reduced very significantly thanks to @mortenpi; the remaining changes should now be reflective of actual AWS changes. @mattBrzezinski, given that the SQS change from QueryService to JSONService is expected, is there anything barring merging this? I suppose there's likely been further changes pushed to this PR since you last looked, do you think you'd have time to give it another look, or is there anything others can do to support that?

Sorry just seeing this now! I can take a pass over this, merge it and tag the new version.

To be honest, I don't have the time day-to-day to work on OSS; nor a desire to spend my free time on it. I am happy to add folks to the organization if they would like to take some ownership over the package, and give an in-depth walkthrough of how it works if desired.

@mattBrzezinski mattBrzezinski merged commit d5052de into master Jun 26, 2024
@mattBrzezinski mattBrzezinski deleted the create-pull-request/patch branch June 26, 2024 14:43
@ararslan
Copy link
Member

Thanks, @mattBrzezinski!

I don't have the time day-to-day to work on OSS

Extremely relatable

I am happy to add folks to the organization if they would like to take some ownership over the package, and give an in-depth walkthrough of how it works if desired.

It could be worth posting a call for maintainers on Discourse and perhaps link to that in the Julia Slack.

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.

4 participants