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

Remove -d flag option from push #64

Closed
ctlong opened this issue Jul 27, 2022 · 3 comments · Fixed by #71
Closed

Remove -d flag option from push #64

ctlong opened this issue Jul 27, 2022 · 3 comments · Fixed by #71

Comments

@ctlong
Copy link
Member

ctlong commented Jul 27, 2022

This flag was deprecated and removed from the cf push command.

case "-d":
app.Routes = append(app.Routes, map[string]string{"route": fmt.Sprintf("%s.%s", appName, args[i+1])})

@ctlong ctlong linked a pull request Sep 27, 2022 that will close this issue
Repository owner moved this from Inbox to Done in App Runtime Deployments Working Group Sep 27, 2022
@ctlong ctlong reopened this Sep 29, 2022
Repository owner moved this from Done to Waiting on feedback in App Runtime Deployments Working Group Sep 29, 2022
@ctlong
Copy link
Member Author

ctlong commented Sep 29, 2022

tldr; this issue was misguided and the PR resolving it introduced a bug into CATs. We should find a way to remove it from cf-test-helpers.

I thought that I was removing the -d flag from cf push with my change, as that flag was deprecated in cf CLI v7. However, the Push function in cf-test-helpers actually generates an application manifest and uses that manifest to cf push the application. Removing the -d option from the Push function removed the option to populate the routes field of the application manifest, which certain CATs isolation segment tests relied upon.

@ctlong
Copy link
Member Author

ctlong commented Sep 29, 2022

My proposal is to revert #71 and cut a patch release. Then to update the release descriptions to make it clear what happened.

If we don't care about the release history, we could also just revert #71 and delete the v2.1.0 release.

ctlong added a commit that referenced this issue Sep 30, 2022
@ctlong
Copy link
Member Author

ctlong commented Sep 30, 2022

The revert commit is included in in the v2.2.0 release.

@ctlong ctlong closed this as completed Sep 30, 2022
Repository owner moved this from Waiting on feedback to Done in App Runtime Deployments Working Group Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

1 participant