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

Use git-submodules for bats-* dependencies #62

Open
hanoii opened this issue Oct 16, 2024 · 3 comments
Open

Use git-submodules for bats-* dependencies #62

hanoii opened this issue Oct 16, 2024 · 3 comments

Comments

@hanoii
Copy link

hanoii commented Oct 16, 2024

This is just a follow up of ddev/github-action-add-on-test#36, and just adding it because I think here is where it should be.

Also from the work on ddev/ddev-drupal-contrib#85 I wanted to add some more thoughts:

On the hate discussion of git submodules I'd like to try to give some perspective, but you need to be open to it...

As I mentioned on ddev/github-action-add-on-test#36, I never liked git submodules either, but I think this is the use case where it thrives.

I tried reading what other people are saying, https://news.ycombinator.com/item?id=31792303 always a good source of opinions, and while one can fine opinions that will resonate with our own either way, I picked up on the following exchange:

Screenshot 2024-10-16 at 09 13 25

What they're really for, is vendoring someone else's code into yours. They're still not great even at that, but sometimes they're the best option.

That's exactly what I am suggesting doing here and what I did on ddev/ddev-drupal-contrib#85 and it's a perfect use case for git submodules. This is not unlike composer.lock or any other vendor lock in dependencies technique. It checks all of the boxes (most of these you have found them on your own tests of that PR):

  • It's their recommended approach on their docs
  • You don't have to install anything, you already have git
  • It's easy to setup
  • It doesn't conflict with any local setup.
  • It works on every architecture as long as you have bash
  • You don't have to account for paths of different ways of having the dependencies.

Yes, it adds the extra burden of maintaining the dependencies lock information, which is the same as any other. This one doesn't change very often though.

Having had the experience of using git submodules for a ddev addon with bats, I stand with my recommendation of favoring that as part of the template.

Feel free to close if you still think otherwise or leave open for further discussions and opinions.

This would fix or make #33 a non-issue.

@rfay
Copy link
Member

rfay commented Oct 16, 2024

Would it solve #33? IIRC the problem there is the "load", not the installation. Installation is easy to do with brew, and brew is already available.

I definitely agree with the potential of submodules, and in your use-case here the only problem is that they require local action for every project on every (new) usage. Those are things I don't need on my machine because I've brew-installed these packages. But it does not add the problem traditional to submodules, which is that every single collaborator gets confused and fails to load them. That's the reason I left them alone years ago.

@hanoii
Copy link
Author

hanoii commented Oct 16, 2024

ON #33, it may be not that it solves it, only that it makes it clearer, it's what we've discussed on ddev/github-action-add-on-test#36 (comment) - what if someone doesn't have them installed with brew?

And yes, it require an action on every new usage, same as with any other project that have dependencies. This is just for running tests locally, so it's not that you need this for "using" the add-on.

@rfay
Copy link
Member

rfay commented Oct 16, 2024

They have to be installed somehow, of course. submodule is one way, brew is another. Regardless, they have to be loaded explicitly.

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

No branches or pull requests

2 participants