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

fix: set process working dir based on relative path of -f file too #296

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ryantm
Copy link
Contributor

@ryantm ryantm commented Jan 9, 2025

Why

  • extends now sets the working dir of the processes in it based on the relative path of the process-compose file.
  • The documentation says that -f should be equivalent to using extends
  • However, -f does not set the working dir of the process to match the process-compose file's path
  • To make the documentation consistent again, we should also set the working dir for the -f filename case.

What changed

  • Move the working dir copy from loadExtendProject to loadProjectFromFile
  • Update test

Test plan

  • -f and extends both set the expected working dir of the processes in my local tests

Why
===
* extends now sets the working dir of the processes in it based on the
relative path of the process-compose file.
* The documentation says that -f should be equvalent to using extends
* However, -f does not set the workign dir of the process to match the
process-compose file's path
* To make the documentation consistent again, we should also set the
working dir for the -f filename case.

What changed
===
* Move the working dir copy from loadExtendProject to
loadProjectFromFile

Test plan
===
* -f and extends both set the expected working dir of the processes in
my local tests
@ryantm ryantm force-pushed the rtm-01-08-fix-working-dir-cli branch from e50ae1a to 8b0f635 Compare January 9, 2025 01:10
Copy link

sonarqubecloud bot commented Jan 9, 2025

@ryantm
Copy link
Contributor Author

ryantm commented Jan 18, 2025

@F1bonacc1 gentle nudge here. Are you reticent about merging this because it breaks some backward compatibility with working dirs of CLI args?

@ryantm
Copy link
Contributor Author

ryantm commented Feb 19, 2025

@F1bonacc1 Just checking in again, it's been a month. Any feedback for this?

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.

1 participant