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: compose flag on okteto up #2804

Merged
merged 4 commits into from
Jun 16, 2022
Merged

Conversation

jLopezbarb
Copy link
Contributor

Signed-off-by: Javier López Barba javier@okteto.com

Fixes #2789

Proposed changes

  • We can't do it as part of serialisation because the docker compose can be in another folder
  • We check if it's in a relative path and if not we create a new subpath based on the sync folder path

Signed-off-by: Javier López Barba <javier@okteto.com>
@jLopezbarb jLopezbarb requested a review from a team June 15, 2022 09:30
@codecov
Copy link

codecov bot commented Jun 15, 2022

Codecov Report

Merging #2804 (b33f252) into master (92d6c7e) will increase coverage by 0.03%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master    #2804      +/-   ##
==========================================
+ Coverage   32.79%   32.83%   +0.03%     
==========================================
  Files         158      158              
  Lines       18614    18626      +12     
==========================================
+ Hits         6105     6115      +10     
- Misses      11785    11786       +1     
- Partials      724      725       +1     
Impacted Files Coverage Δ
pkg/model/dev.go 70.14% <84.61%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92d6c7e...b33f252. Read the comment docs.

Signed-off-by: Javier López Barba <javier@okteto.com>
pkg/model/dev.go Outdated Show resolved Hide resolved
pkg/model/dev.go Outdated Show resolved Hide resolved
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
pkg/model/dev.go Show resolved Hide resolved
@@ -1019,6 +1020,21 @@ func (dev *Dev) ToTranslationRule(main *Dev, reset bool) *TranslationRule {

return rule
}
func getSubPathFromLocalPath(localPath string) string {
wd, err := os.Getwd()
Copy link
Contributor

Choose a reason for hiding this comment

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

can we inject our dependencies for reading the file system? This will make unit testing much easier.

Optimally we would inject it at the top of our command, but for now to get this function's tests easier we can create the file reading dependencies in the caller

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That requires a whole refactor, which will be made in a future, so I think it will be better to work on it when we do the big refactor

@jLopezbarb jLopezbarb requested a review from ifbyol June 15, 2022 14:26
@jLopezbarb jLopezbarb merged commit 8d63511 into master Jun 16, 2022
@jLopezbarb jLopezbarb deleted the jlopezbarb/fix-compose-flag branch June 16, 2022 07:11
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
* fix: compose flag on okteto up

Signed-off-by: Javier López Barba <javier@okteto.com>

* test: add test for non relative path

Signed-off-by: Javier López Barba <javier@okteto.com>

* refactor: encapsulate code into a function

Signed-off-by: Javier López Barba <javier@okteto.com>

* fix: unit tests

Signed-off-by: Javier López Barba <javier@okteto.com>
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
* fix: compose flag on okteto up

Signed-off-by: Javier López Barba <javier@okteto.com>

* test: add test for non relative path

Signed-off-by: Javier López Barba <javier@okteto.com>

* refactor: encapsulate code into a function

Signed-off-by: Javier López Barba <javier@okteto.com>

* fix: unit tests

Signed-off-by: Javier López Barba <javier@okteto.com>
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
* fix: compose flag on okteto up

Signed-off-by: Javier López Barba <javier@okteto.com>

* test: add test for non relative path

Signed-off-by: Javier López Barba <javier@okteto.com>

* refactor: encapsulate code into a function

Signed-off-by: Javier López Barba <javier@okteto.com>

* fix: unit tests

Signed-off-by: Javier López Barba <javier@okteto.com>
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
This reverts commit 8d63511.

Signed-off-by: Javier López Barba <javier@okteto.com>
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
This reverts commit 8d63511.

Signed-off-by: Javier López Barba <javier@okteto.com>
jLopezbarb added a commit that referenced this pull request Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

okteto up -f is working different from okteto up on compose
4 participants