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.

Sign up for GitHub

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

Jump to bottom

feat: Add optional gzip flag #15

Merged
merged 1 commit into from
Feb 4, 2021
Merged

feat: Add optional gzip flag #15

merged 1 commit into from
Feb 4, 2021

Conversation

omrihq
Copy link
Contributor

@omrihq omrihq commented Feb 2, 2021

No description provided.

@google-cla google-cla bot added the cla: yes label Feb 2, 2021
@omrihq omrihq changed the title Add optional gzip flag feat-req: Add optional gzip flag Feb 2, 2021
@github-actions github-actions bot added the Fork label Feb 2, 2021
@omrihq omrihq force-pushed the oh-gzip branch 5 times, most recently from 3609cd0 to c71f870 Compare February 2, 2021 00:31
@omrihq omrihq mentioned this pull request Feb 2, 2021
Closed
@omrihq
Copy link
Contributor Author

omrihq commented Feb 2, 2021

Hi @bharathkkb and @averikitsch, thank you for this github action, it's a savior, and this is the last bit needed for a use-case we have for uploading to GCS. Let me know if I should leave this PR here or in https://github.com/google-github-actions/setup-gcloud/pulls

Thank you!

@averikitsch
Copy link
Contributor

Hi @omrihq

Let me know if I should leave this PR here or in https://github.com/google-github-actions/setup-gcloud/pulls

This is the correct place. Thanks for the PR we will review shortly.

averikitsch
averikitsch suggested changes Feb 3, 2021
View reviewed changes
Copy link
Contributor

@averikitsch averikitsch left a comment

Choose a reason for hiding this comment

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

Please update main.ts to get the action input for this flag

README.md Outdated Show resolved Hide resolved
src/upload-helper.ts Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@omrihq omrihq force-pushed the oh-gzip branch 4 times, most recently from 4ce4ed6 to 622582a Compare February 3, 2021 18:39
@omrihq
Copy link
Contributor Author

omrihq commented Feb 3, 2021

Thanks for the review @averikitsch, updated with all your suggestions

averikitsch
averikitsch reviewed Feb 4, 2021
View reviewed changes
Copy link
Contributor

@averikitsch averikitsch left a comment

Choose a reason for hiding this comment

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

@bharathkkb do you have any feedback?

src/upload-helper.ts Outdated Show resolved Hide resolved
@bharathkkb
Copy link
Contributor

@omrihq Thanks for the PR! Overall LGTM after @averikitsch feedback

@omrihq omrihq force-pushed the oh-gzip branch 7 times, most recently from abd1f6e to 14bdc1c Compare February 4, 2021 16:48
@omrihq omrihq force-pushed the oh-gzip branch 10 times, most recently from 1192595 to 62ce704 Compare February 4, 2021 17:28
@omrihq
Copy link
Contributor Author

omrihq commented Feb 4, 2021

@averikitsch Changing gzip to be required in upload-helper.ts had quite a few cascading changes, such as reordering the directory/gzip params. Thanks for the review!

@averikitsch averikitsch merged commit 2f6066a into google-github-actions:main Feb 4, 2021
@google-github-actions-bot google-github-actions-bot mentioned this pull request Feb 4, 2021
Merged
@omrihq omrihq deleted the oh-gzip branch February 4, 2021 19:21
@omrihq omrihq mentioned this pull request Feb 4, 2021
Closed
@averikitsch averikitsch changed the title feat-req: Add optional gzip flag feat: Add optional gzip flag Feb 4, 2021
@bharathkkb bharathkkb mentioned this pull request Feb 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@averikitsch averikitsch averikitsch approved these changes

@bharathkkb bharathkkb Awaiting requested review from bharathkkb

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@omrihq @averikitsch @bharathkkb