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

feat: support google cloud storage as das #2643

Merged
merged 19 commits into from
Oct 7, 2024

Conversation

renlulu
Copy link
Contributor

@renlulu renlulu commented Sep 6, 2024

No description provided.

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Sep 6, 2024
Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Nice work so far! It just needs some additional testing and addressing my comments, which are fairly minor.

das/google_cloud_storage_service.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
das/google_cloud_storage_service.go Outdated Show resolved Hide resolved
@renlulu
Copy link
Contributor Author

renlulu commented Sep 7, 2024

Proof of working:
image
image

@renlulu renlulu force-pushed the feat/das-google-cloud branch 2 times, most recently from a9aee5f to 0750428 Compare September 9, 2024 19:52
das/google_cloud_storage_service.go Outdated Show resolved Hide resolved
das/google_cloud_storage_service.go Outdated Show resolved Hide resolved
cmd/genericconf/config.go Outdated Show resolved Hide resolved
cmd/genericconf/config.go Outdated Show resolved Hide resolved
@renlulu
Copy link
Contributor Author

renlulu commented Sep 20, 2024

@Tristan-Wilson Hey can we follow up here?

Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Looking good, minor comment only now.

cmd/genericconf/config.go Outdated Show resolved Hide resolved
@renlulu
Copy link
Contributor Author

renlulu commented Oct 2, 2024

Looking good, minor comment only now.

Sorry i just saw the comment, I adjust the PR per your request.

Tristan-Wilson
Tristan-Wilson previously approved these changes Oct 3, 2024
Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

LGTM. You will need to wait now for one of my colleagues to give this the design-reviewed tag.

Copy link
Contributor

@tsahee tsahee left a comment

Choose a reason for hiding this comment

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

Approving based on Tristan's review to allow CI

@tsahee tsahee merged commit 79d6ec8 into OffchainLabs:master Oct 7, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-approved s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants