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

docker-compose.override.yml file breaks the compose sub cmd #252

Closed
carlosms opened this issue Sep 30, 2019 · 6 comments · Fixed by #262
Closed

docker-compose.override.yml file breaks the compose sub cmd #252

carlosms opened this issue Sep 30, 2019 · 6 comments · Fixed by #262
Assignees
Labels
enhancement New feature or request

Comments

@carlosms
Copy link
Contributor

The override file must match the compose file format version of the main docker-compose.yml file.
While the main docker-compose.yml file is managed by sourced compose, the contents of the docker-compose.override.yml file are hardcoded into the sourced bin.

See for example the upgrading instructions here https://github.com/src-d/sourced-ce/pull/251/files

Why is the docker-compose.override.yml file needed at all in sourced? Can we find a way to manage this cleanly from the preparation scripts of sourced-ui instead?

@smacker
Copy link
Contributor

smacker commented Oct 17, 2019

I think we can remove everything about override from srcd-ce.
If a developer needs an override for a new workdir he/she can just execute make set-override in srcd-ui again. What is your opinion @se7entyse7en ?

@se7entyse7en
Copy link
Contributor

I agree. I also actually never used docker-compose.override.yml and also even did't remember why we added it. Moreover, it's an internal thing that has an impact to the end-user as it makes updating less friendly (even if it's just a matter of an additional instruction).

@se7entyse7en
Copy link
Contributor

@dpordomingo wdyt?

@se7entyse7en se7entyse7en added enhancement New feature or request needs discussion/agreement This issue needs some discussion or agreement labels Oct 18, 2019
@dpordomingo
Copy link
Contributor

In the PR #158 which added it, I initially implemented the safety net to ensure that it wouldn't collide with any user state. It was suggested to make it static, and I did so.
🤷‍♂️

Yes, we can also delete it and take another approach. There are always different alternatives, and considering our current state, maybe that's the fastest one.

@se7entyse7en
Copy link
Contributor

Okay, let's remove it then.

@se7entyse7en se7entyse7en removed the needs discussion/agreement This issue needs some discussion or agreement label Oct 22, 2019
@se7entyse7en se7entyse7en self-assigned this Oct 25, 2019
@dpordomingo
Copy link
Contributor

This caused src-d/sourced-ui#318

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants