Skip to content

+aws-sdk-cpp#1828

Merged
jhheider merged 5 commits into
pkgxdev:mainfrom
JrGoodle:+aws-sdk-cpp
May 20, 2023
Merged

+aws-sdk-cpp#1828
jhheider merged 5 commits into
pkgxdev:mainfrom
JrGoodle:+aws-sdk-cpp

Conversation

@JrGoodle
Copy link
Copy Markdown
Contributor

@JrGoodle JrGoodle commented May 4, 2023

closes #545

@what-the-diff
Copy link
Copy Markdown

what-the-diff Bot commented May 4, 2023

PR Summary

  • Added a new file
    A new file was introduced to the repository.
  • Changed URL to GitHub releases
    The URL now points to GitHub releases instead of the master branch, improving stability.
  • Replaced submodule update with direct cloning
    The build script no longer utilizes git submodules, opting to clone the aws-crt-cpp repo directly into crt/aws-crt-cpp directory for a more straightforward workflow.

@JrGoodle
Copy link
Copy Markdown
Contributor Author

JrGoodle commented May 4, 2023

seems like this is gonna be a fun one. there's a submodule that's not included in the tarball. and since there's no .git directory, it's not clear what commit of the submodule is needed. and it's recursive, so the submodule has submodules.

https://github.com/aws/aws-sdk-cpp/blob/main/.gitmodules
https://github.com/aws/aws-sdk-cpp/tree/main/crt
https://github.com/awslabs/aws-crt-cpp/blob/cb474daeeaf5c025bd3408103adf61b97b74e600/.gitmodules

I could imagine just cloning the repo instead of grabbing the tarball. or using the github api to figure out the submodule commit hash, then cloning the correct commit. or getting the commit and then getting a tarball, but you'd have to do that recursively.

I searched for "submodule" in the pantry repo and didn't see any existing references, so I dunno if this case has come up yet.

@JrGoodle
Copy link
Copy Markdown
Contributor Author

JrGoodle commented May 6, 2023

I think the submodule issue here likely needs to be solved by: pkgxdev/brewkit#111

this works with the upcoming brewkit 0.31.0
@jhheider
Copy link
Copy Markdown
Contributor

the fixes added work for me with the upcoming brewkit 0.31.0. recommend renaming to aws.amazon.com/aws-sdk-cpp.

@jhheider
Copy link
Copy Markdown
Contributor

there we go

@mxcl
Copy link
Copy Markdown
Contributor

mxcl commented May 16, 2023

Still marked as draft. Deliberate?

@jhheider
Copy link
Copy Markdown
Contributor

gonna go ahead and merge this. we can open for new work if needed.

@jhheider jhheider marked this pull request as ready for review May 20, 2023 04:02
@jhheider jhheider merged commit 9378f80 into pkgxdev:main May 20, 2023
@JrGoodle
Copy link
Copy Markdown
Contributor Author

sorry, got a bit sidetracked by work and life. thanks for the followup. I'll try to get to my other straggling PR drafts this week.

btw @jhheider packaging up the git repo as a tarball so it would "just work" with the rest of the brewkit code was pretty clever. when I was looking at the code that's where I got kinda overwhelmed by the changes that would have been needed without that trick.

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.

+aws-sdk-cpp (173/548)

3 participants