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.

Sign up for GitHub

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

Jump to bottom

Issue 3373 #268

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Issue 3373 #268

wants to merge 6 commits into from

Conversation

florianPat
Copy link

Hey all,
this pull request resolves the lando/lando issue #3373 (lando/lando#3373).
It does so by using the "docker compose config" command to resolve all paths relative to the lando compose file root.
I also have some other (opinionated) changes in this PR, so please let me know what you think about them.
Feel free to just drop commits that do not work (my guess would be the docker compose v2 seperator change could be such a thing, as its not backwards compatible?)

Would love a conversation on the changes or if I maybe should start with a smaller pull request will less changes?

We are using this feature extensively in our project at work and I would love to get this merged. Currently, I am maintaining a fork. Maybe some of the features could also implemented as a plugin, did not really think about that till now.

Thanks for any feedback,
Flo

@netlify Netlify
Copy link

netlify bot commented Nov 9, 2024 •
edited
Loading

👷 Deploy request for lando-core pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit b7399e3

@florianPat florianPat force-pushed the issue-3373 branch 5 times, most recently from 44eccd4 to bdc71ea Compare November 16, 2024 08:19
@florianPat
Copy link
Author

Hey @reynoldsalec,
I just noticed that the core tests were not passing because I do not have access to the secrets?
I just ran the tests in a pull request from my repo (where the secret is commented out) and they pass. So, not all of them, because for the plugin loading secrets are needed, but for all other ones :D
See here: PR
Do I have to configure something on my part?
Would love to get some feedback on this one!

Thanks,
Flo

@florianPat florianPat force-pushed the issue-3373 branch 4 times, most recently from 4e56e47 to c39d2dd Compare December 29, 2024 12:55
@florianPat
Copy link
Author

Hey @pirog,
I just updated the PR to just fix the compose service issue AND fixing other small things I noticed working with this:

  • permission setup for alpine containers
  • using the app_mount if that was set in the .lando.yml
  • also be able to override the core plugin so that people can just install another one to develop on easily

The github action still needs to run.
Please have a look at it.

Thanks in advance,
Flo

@florianPat
Copy link
Author

Note that I just ran the tests and all are passing which do not need github authentication or the key pair: https://github.com/florianPat/lando-core/pull/6/checks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@florianPat