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(xml): add support for list objects #1115

Merged
merged 1 commit into from
May 9, 2023

Conversation

goober
Copy link
Contributor

@goober goober commented Apr 2, 2023

This pull request aims to add support for listing objects according to the Google Cloud Storage XML API

Copy link
Owner

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

Hey, thanks for contributing. I'm planning to have a look at this later today, can you rebase to fix the conflict? Thank you!

@fsouza fsouza force-pushed the feature/xml-list-objects branch from a36199c to 9a77d53 Compare May 3, 2023 02:32
Copy link
Owner

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

I pushed some changes to your branch to get the PR green, but feel free to override them.

I think this looks good overall, but I'd like to have the pagination change as a separate PR or available only through the HTTP API, we don't need to expose pagination as part of the library surface (like let's implement it in an unexported method that gets used by ListObjectsWithOptions).

fakestorage/object.go Outdated Show resolved Hide resolved
@goober goober force-pushed the feature/xml-list-objects branch from 71dc486 to 7e56881 Compare May 8, 2023 08:31
@goober
Copy link
Contributor Author

goober commented May 8, 2023

Thanks for the review @fsouza, and I agree that we should strive for not breaking the current api. So I have removed everything that is related to pagination in the current PR. And then we could have another look at that in a separate PR later on, if necessary.

@goober goober force-pushed the feature/xml-list-objects branch from 7e56881 to be4a0d6 Compare May 8, 2023 08:35
Copy link
Owner

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

Thank you!

@fsouza fsouza merged commit 0bcd64c into fsouza:main May 9, 2023
@goober goober deleted the feature/xml-list-objects branch May 9, 2023 09:17
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