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

Migration Issue fix for Google API v1beta to v1 #206

Merged
merged 18 commits into from
Apr 1, 2020

Conversation

bao1018
Copy link
Contributor

@bao1018 bao1018 commented Mar 31, 2020

couple of changes base on v2.4.2 this PR is going to fix below issue:#202

  1. remove location property
  2. set default region property value
  3. add migration guide for v1beta to v1

@bao1018 bao1018 requested a review from medikoo March 31, 2020 09:41
@bao1018 bao1018 changed the title Migration Issue fix for Google API v1beta > v1 Migration Issue fix for Google API v1beta to v1 Mar 31, 2020
@bao1018
Copy link
Contributor Author

bao1018 commented Mar 31, 2020

@medikoo thanks for your patience, this is the fully tested version, you can use your proposed process to publish the new release. thanks

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@bao1018 Great thanks, it looks great. Still I think we still need to fix v2 (do not leave it broken) therefore I think steps should be following:

  1. Prepare revert of Migrate to CloudFunctions v1 API. #165 PR (you can easily create such PR, by visiting this PR and clicking "Revert") and merge it to master
  2. Prepare v2.4.3 release PR (please get familiar with https://github.com/serverless/serverless-google-cloudfunctions/blob/master/RELEASE_PROCESS.md) and merge (after merge v2.4.3 will be automatically released)
  3. Update this PR to also have changes that were proposed in Migrate to CloudFunctions v1 API. #165 (but do not bump release, just in commit message indicate it's a BREAKING CHANGE)
  4. Upgrade of googleapis (we're not using latest version here), and drop of support for Node.js v6 (another BREAKING CHANGE commit).
  5. Prepare v3.0.0 release PR (after merge v3.0.0 will be automatically published)

If you need help with anything, let me know

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "serverless-google-cloudfunctions",
"version": "2.4.2",
"version": "3.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should done with dedicated release PR. Please get familiar with https://github.com/serverless/serverless-google-cloudfunctions/blob/master/RELEASE_PROCESS.md

@bao1018
Copy link
Contributor Author

bao1018 commented Apr 1, 2020

@zxhaaa6 can you address medikoo comments as soon as possible ? thanks

@zxhaaa6
Copy link
Contributor

zxhaaa6 commented Apr 1, 2020

@bao1018 sure, thanks

@zxhaaa6
Copy link
Contributor

zxhaaa6 commented Apr 1, 2020

Hi @medikoo, could you please help to review? And after that, I would prepare v3.0.0 release PR. Many thanks!

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Great thanks @zxhaaa6 !

Either this work should be split into two PR's as follows:

  1. Change that was proposed with Migrate to CloudFunctions v1 API. #165
  2. Upgrade of googleapis (so drop of support for Node.js)

Or it can stay in one PR, but then change should be proposed with two distinct (rebased on master, with no merge commit) and well described commits, with messages that follow Commit Message Guidelines.

Both ways are fine for me, so pick what feels better for you.

It's the only way to maintain history of repo clean, and to automatically produce a valid release

@zxhaaa6
Copy link
Contributor

zxhaaa6 commented Apr 1, 2020

Oh sorry, I will split Upgrade of googleapis (so drop of support for Node.js) into another PR. Thanks @medikoo

@zxhaaa6
Copy link
Contributor

zxhaaa6 commented Apr 1, 2020

Hi @medikoo , could you please help to review? After that, I will do next step. Thanks!

@zxhaaa6
Copy link
Contributor

zxhaaa6 commented Apr 1, 2020

@medikoo , a strange travis issue

@medikoo medikoo merged commit 482ee0e into serverless:master Apr 1, 2020
@medikoo
Copy link
Contributor

medikoo commented Apr 1, 2020

@medikoo , a strange travis issue

Yes I saw, I've merged it as it didn't seem related to code changes

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.

4 participants