-
Notifications
You must be signed in to change notification settings - Fork 3
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
Accept --location option #105
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small thing, but need to change it in multiple places.
7115de0
to
257b551
Compare
Known issue: Passing non-exisiting location causes error:
|
Let's also add Justin's comment from https://github.com/shakacode/heroku-to-control-plane/pull/92/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R172 here, since this PR is adding the option. |
@rafaelgomesxyz Should we check with the ControlPlane actual deployment to ensure the location we use matches the location of the deployed project? I guess there should be multiple edge cases. We may need to discuss or address those situations in later PRs. What do you think? Notice: This PR's implementation may be affected by #110 implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rafaelgomesxyz Should we check with the ControlPlane actual deployment to ensure the location we use matches the location of the deployed project?
Maybe in another PR, I don't think it fits into the scope of this PR.
This commit is a fix for passing tests.
51b5e6c
to
b024d77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes.
This PR add a
-l, --location
option to the project, which can be used in the following commands:apply-template
ps
run
run:detached
Question: Should we have--> Added in ad26c1eCPLN_LOCATION
as well?Question: Should we add an option to prevent override by env variable? (like org and app)
Closes #100