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

Make storage bucket parameterizeable. Create const for bucket picker #1518

Merged
merged 9 commits into from
Feb 7, 2024

Conversation

inlined
Copy link
Member

@inlined inlined commented Jan 31, 2024

API spec basically said to parameterize everything so I don't know why this was overlooked. Also, I've added a const BUCKET_PICKER so customers don't have to say { input: { resource: { type: "storage.googleapis.com/Bucket" } } } and can instead say { input: BUCKET_PICKER }. That const might need to go through API council review though.

@inlined inlined requested a review from mbleigh January 31, 2024 02:01
Comment on lines +212 to +217
export const BUCKET_PICKER: ResourceInput = {
resource: {
type: "storage.googleapis.com/Bucket",
},
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the helper constant even needed given VS Code will autocomplete the enum string for the resource type?

If so, might prefer naming of SELECT_STORAGE_BUCKET over BUCKET_PICKER.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is much less cringe to type a constant rather than a complex structure that has only one possible configuration. There's precedence here with the ServerValues consts/functions in RTDB/Firestore. Switched to your preferred name though.

@inlined
Copy link
Member Author

inlined commented Feb 2, 2024

Given the additional functions added, there is no question that this now needs API council review. The review is scheduled for next week.

@inlined inlined merged commit 29fc722 into master Feb 7, 2024
13 checks passed
@inlined inlined deleted the inlined.bucket-params branch February 8, 2024 00:41
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