Skip to content
This repository was archived by the owner on Nov 27, 2024. It is now read-only.

Check folder change before using v3 API#770

Merged
iyabchen merged 3 commits into
GoogleCloudPlatform:mainfrom
iyabchen:ancestry-v1-patch
Jun 15, 2022
Merged

Check folder change before using v3 API#770
iyabchen merged 3 commits into
GoogleCloudPlatform:mainfrom
iyabchen:ancestry-v1-patch

Conversation

@iyabchen

Copy link
Copy Markdown
Contributor

No description provided.

@iyabchen iyabchen marked this pull request as ready for review June 15, 2022 18:08
@iyabchen iyabchen requested review from a team and melinath and removed request for a team June 15, 2022 18:08
Comment thread ancestrymanager/ancestrymanager.go Outdated
Comment thread ancestrymanager/ancestrymanager_test.go

@melinath melinath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM apart from a few minor fixes to the names for the new tests & logic for one.

Comment thread ancestrymanager/ancestrymanager_test.go Outdated
Comment thread ancestrymanager/ancestrymanager_test.go Outdated
Comment thread ancestrymanager/ancestrymanager_test.go Outdated
Comment thread ancestrymanager/ancestrymanager_test.go Outdated
Comment thread ancestrymanager/ancestrymanager_test.go Outdated

@melinath melinath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

v1Count: 1,
},
{
// project moving from organizations/qux/folders/bar to organizations/qux2/folders/bar2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

💯 great to have this added information in the comments!

@iyabchen iyabchen merged commit b6f2527 into GoogleCloudPlatform:main Jun 15, 2022
melinath pushed a commit that referenced this pull request Jun 16, 2022
* Check folder change before using v3 API

* address case to move folder between parent and child

* Fix wrong test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants