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: Add move-route command to cf cli #2302

Merged
merged 20 commits into from
Aug 23, 2022
Merged

Conversation

PeteLevineA
Copy link
Member

Co-authored-by: Shwetha Gururaj [email protected]

This is an intro PR for the upcoming changes made by the platform interfaces team. This PR will not be merged until sign off and successful run of integration test below once we have an environment with cf-api:edge.

Does this PR modify CLI v6, CLI v7, or CLI v8?

v8

Description of the Change

Adding the cf transfer-route-owner command to the cf cli

Why Is This PR Valuable?

When users share a route, ownership is maintained by the originating space. This new command allows transfer of ownership.

Why Should This Be In Core?

This command is needed when a route is shared and needs to be deleted or modified by the target space.

Applicable Issues

List any applicable Github Issues here

How Urgent Is The Change?

Not urgent, still working with the Platform Interface team on definition.

Other Relevant Parties

CF API transfer route owner endpoint is required

Copy link
Member

@a-b a-b left a comment

Choose a reason for hiding this comment

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

LGTM

@Gerg
Copy link
Member

Gerg commented Aug 3, 2022

I may be behind the times, but I think we decided to change this command to move-route.

cc @MerricdeLauney

@MerricdeLauney
Copy link
Member

I may be behind the times, but I think we decided to change this command to move-route.

cc @MerricdeLauney

Based on this conversation i believe you are correct

Co-authored-by: Shwetha Gururaj <[email protected]>
@PeteLevineA PeteLevineA changed the title feat: Add transfer-route-owner command to cf cli feat: Add move-route command to cf cli Aug 4, 2022
@PeteLevineA
Copy link
Member Author

thanks @Gerg & @MerricdeLauney . @gururajsh and I updated to move-route. Completely forgot about that thread.

Copy link
Contributor

@jdgonzaleza jdgonzaleza left a comment

Choose a reason for hiding this comment

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

LGTM

@gururajsh gururajsh merged commit 68d897a into master Aug 23, 2022
@gururajsh gururajsh deleted the feature/tas-e-259-ownership branch August 23, 2022 18:03
gururajsh added a commit that referenced this pull request Aug 23, 2022
* feat: Add transfer-route-owner command to cf cli

Co-authored-by: Shwetha Gururaj <[email protected]>

* feat: update repository variable. Update workflow action

* fix: remove windows 2016, swap to windows latest. Fix relative file path

* chore: windows values

* chore: swap to Rel instead of join

* chore: main dir

* chore: join on current dir

* chore: clean path first

* chore: try from slash

* chore: remove dot syntax

* chore: swap to move-route

Co-authored-by: Shwetha Gururaj <[email protected]>

* chore: windows tests again

* chore: remove test println

* chore: update path to /relationships/space from transferowner

Co-authored-by: Shwetha Gururaj <[email protected]>

* Change display text

* Remove focus

* Fix typo

* Send non-array data in requestBody

* Change message ordering

* Create shared domain instead of private

Co-authored-by: Shwetha Gururaj <[email protected]>
@@ -12,7 +12,7 @@ jobs:
analyze:
name: Analyze
runs-on: ubuntu-latest
if: ${{ github.action_repository == 'cloudfoundry/cli' }}
if: ${{ github.repository == 'cloudfoundry/cli' }}
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer having GitHub Actions related changes in a separate PR, so we can easily synchronize these changes with other branches.

@@ -83,9 +79,8 @@ jobs:
strategy:
matrix:
os:
- windows-2022
- windows-latest
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why we unlocked this.

gururajsh added a commit that referenced this pull request Aug 24, 2022
* feat: Add transfer-route-owner command to cf cli

Co-authored-by: Shwetha Gururaj <[email protected]>

* feat: update repository variable. Update workflow action

* fix: remove windows 2016, swap to windows latest. Fix relative file path

* chore: windows values

* chore: swap to Rel instead of join

* chore: main dir

* chore: join on current dir

* chore: clean path first

* chore: try from slash

* chore: remove dot syntax

* chore: swap to move-route

Co-authored-by: Shwetha Gururaj <[email protected]>

* chore: windows tests again

* chore: remove test println

* chore: update path to /relationships/space from transferowner

Co-authored-by: Shwetha Gururaj <[email protected]>

* Change display text

* Remove focus

* Fix typo

* Send non-array data in requestBody

* Change message ordering

* Create shared domain instead of private

Co-authored-by: Shwetha Gururaj <[email protected]>

Co-authored-by: Peter Levine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants