starshot-prototype icon indicating copy to clipboard operation
starshot-prototype copied to clipboard

Make it extremely easy to boot up Starshot with LANDO

Open MatthieuScarset opened this issue 9 months ago • 12 comments

Following the example in #50 I added a .lando.yml file to help users who prefer this tool, making it ridiculously easy to set up Starshot with LANDO.

This PR adds that to the README too:

lando start && lando install && lando login

MatthieuScarset avatar May 08 '24 19:05 MatthieuScarset

Hmmm...I tried to test this PR locally and as soon as I cloned it and ran lando start, I hit this:

Unhandled rejection TypeError: Cannot read properties of null (reading 'trim')
    at Object.clean (/snapshot/cli/node_modules/@lando/core/node_modules/semver/functions/clean.js)
    at /snapshot/cli/node_modules/@lando/core/utils/is-compatible-version.js
    at /snapshot/cli/node_modules/@lando/core/lib/engine.js
    at /snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js
    at /snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js
    at baseForOwn (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at /snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js
    at baseMap (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at Function.map (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at interceptor (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at Function.thru (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at /snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js
    at arrayReduce (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at baseWrapperValue (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at LodashWrapper.wrapperValue (/snapshot/cli/node_modules/@lando/core/node_modules/lodash/lodash.js)
    at /snapshot/cli/node_modules/@lando/core/lib/engine.js
From previous event:
    at Engine.getCompatibility (/snapshot/cli/node_modules/@lando/core/lib/engine.js)
    at /snapshot/cli/node_modules/@lando/core/hooks/lando-get-compat.js
    at AsyncEvents.<anonymous> (/snapshot/cli/node_modules/@lando/core/index.js)
    at AsyncEvents.handle (/snapshot/cli/node_modules/@lando/core/lib/events.js)
    at /snapshot/cli/node_modules/@lando/core/lib/events.js
From previous event:
    at AsyncEvents.emit (/snapshot/cli/node_modules/@lando/core/lib/events.js)
    at /snapshot/cli/node_modules/@lando/core/lib/lando.js
    at process.processImmediate (node:internal/timers:476:21)
From previous event:
    at Lando.bootstrap (/snapshot/cli/node_modules/@lando/core/lib/lando.js)
    at Object.<anonymous> (/snapshot/cli/bin/lando)
    at Module._compile (pkg/prelude/bootstrap.js:1930:22)
    at Module._extensions..js (node:internal/modules/cjs/loader:1414:10)
    at Module.load (node:internal/modules/cjs/loader:1197:32)
    at Module._load (node:internal/modules/cjs/loader:1013:12)
    at Function.runMain (pkg/prelude/bootstrap.js:1983:12)
    at node:internal/main/run_main_module:28:49

Let's get this party started! Starting app starshot...
ERROR ==> Cannot read properties of null (reading 'trim')

Running lando version reveals v3.21.0-beta.18. Any ideas?

phenaproxima avatar May 09 '24 02:05 phenaproxima

Hmmm...I tried to test this PR locally and as soon as I cloned it and ran lando start, I hit this:

Sorry for this issue. I guess it is caused by some wrong indentation I had in the .lando.yml file maybe :shrug:

I've updated this file and it should be working fine now.

I've tested on a fresh app with lando destroy -y && lando start && lando drupal:install

MatthieuScarset avatar May 10 '24 19:05 MatthieuScarset

I checked out this branch and ran the lando commands from the readme and everything worked great! lando start && lando drupal:install

Thank you for this!

Taiger avatar May 10 '24 21:05 Taiger

@phenaproxima so DDEV was added, now Lando. There are already issues opened for DDEV not working.

Is this really necessary to add these unrelated tools to this repo and then support them?

By providing support for these tools, unrelated to actual project functionality, the resources are spent on these things instead of the product itself.

AlexSkrypnyk avatar May 10 '24 23:05 AlexSkrypnyk

Providing an initial setup for DDEV and Lando is important. Arguably, as important as Drush or Composer tooling.

I was tempted to suggest additional changes for Lando but this initial setup works great as is. I feel like the DDEV and Lando setups should be barebones because developers will need to change them regardless.

It seems like the aim of of this entire project, is to have better initial defaults. Something that Drupal has struggled with over the years.

Taiger avatar May 11 '24 16:05 Taiger

@AlexSkrypnyk, I frankly agree with @Taiger on this one.

Why not make it as easy as possible for as many people as possible? It doesn't cost us much, and it makes Starshot more accessible. We're not doing extensive support for Lando or DDEV -- just making sure it's easy to spin up. So far, that seems to be working out pretty well, and has been received positively.

I am +1 for this and will merge it once everything is sorted out.

phenaproxima avatar May 13 '24 14:05 phenaproxima

@AlexSkrypnyk, I don't understand your opposition to adding DDEV and Lando support. Why not give developers, who want to work on the project, and who use DDEV and Lando (the two most popular Drupal developer tools) a head start? With Lando and DDEV support out-of-the-box, developers using these tools can get up and running much, much faster and start working on the project, and improving it.

Also, with support for Lando and DDEV, evaluators using these tools can much more easily spin up an instance of Starshot, and check it out.

gitressa avatar May 13 '24 17:05 gitressa

@gitressa I’m not opposed of adding these great tools. I’m simply asking a question to bring the awareness of the maintenance overhead. If maintainers are okay to take it on themselves - it’s great!

@phenaproxima I think your comment answers my question about supporting these tools. Thank you

AlexSkrypnyk avatar May 13 '24 20:05 AlexSkrypnyk

In order to provide an easy starting point for users who haven't decided on which development environment to use, we have decided to recommend DDEV. You can read https://www.drupal.org/project/ideas/issues/2965681 for context. We should honor that decision here, and instead of providing several options, we should make it clear how users can get started with the recommended local development environment. It makes sense to have a Lando file somewhere that users who are already using Lando can use, but we need to be careful not to do that with a cost to majority of the users.

lauriii avatar May 23 '24 13:05 lauriii

In order to provide an easy starting point for users who haven't decided on which development environment to use, we have decided to recommend DDEV. You can read https://www.drupal.org/project/ideas/issues/2965681 for context. We should honor that decision here, and instead of providing several options, we should make it clear how users can get started with the recommended local development environment. It makes sense to have a Lando file somewhere that users who are already using Lando can use, but we need to be careful not to do that with a cost to majority of the users.

Thank you for pointing out this d.o issue. I didn't know about it.

As I commented there, I'm surprised we want to skip 50% of our dev community who currently use Lando.

Adding a .lando.yml file is non-intrusive, very easy to maintain and helps a broader user base.

I'd like to request a new review to make it extremely easy to boot up Starshot with Lando.


(edit for clarity) The issue with .drush.yml file is the only thing left to fix: https://github.com/phenaproxima/starshot-prototype/pull/55#discussion_r1598976807

Otherwise just run lando start && lando drupal:install and done!

MatthieuScarset avatar May 24 '24 04:05 MatthieuScarset

@MatthieuScarset, I tried to review this but for some reason, Lando is always giving me this error:

ERROR ==> Cannot read properties of null (reading 'trim')

...and refusing to go any further, if I run lando start having checked out your PR branch. Any advice here? I need to manually test this before I merge it. lando version reports v3.21.0-beta.18.

phenaproxima avatar May 25 '24 02:05 phenaproxima

um... I guess this is something with your local Lando env.

I've destroyed mine. Removed the folder. Git clone and lando start && lando drupal:install and it all worked.

My lando version is v3.21.0-beta.20

image

MatthieuScarset avatar May 25 '24 14:05 MatthieuScarset

It worked for me too without any error. I just cloned the repo and run lando start && lando drupal:install. Everything seems to be working perfectly.

My lando version is v3.18.0

image

Manish-Sharma1995 avatar May 29 '24 09:05 Manish-Sharma1995

Having pondered this for some time, I'm not really convinced we should merge this. My reasons:

  • It's taken a lot of iterating and fiddling around to get DDEV support to where it is, and adding support for a whole other environment with its own idiosyncrasies means there is more we need to maintain. (Right now, we have to effectively support two development environments: DDEV, and bare metal. That's already a lot.)
  • DDEV is the officially recommended development environment for Drupal sites. Whether you agree or disagree with that status, it wasn't my decision; Lando is undoubtedly an awesome tool, and I have nothing against it, but it's not the one that seems to be slated for official, wide support in the Drupal community. This is anecdotally backed up by the fact that I've seen a lot of activity in this repo concerning DDEV, and only this PR regarding Lando.
  • I'd like to keep the support for various development environments as consistent as possible. If we add Lando support now, then maybe we'll add something later for DDEV and then not be able to supply something similar for Lando, due to whatever unforeseen technical limitations. The maintenance load and added complexity doesn't seem worthwhile to me.
  • I've not been able to test it. Lando is installed on my machine but stubbornly gives me the error I mentioned in https://github.com/phenaproxima/starshot-prototype/pull/55#issuecomment-2130663416.

For these reasons, I think I'm closing this for now as a wontfix. I think we should revisit this after the "real" (non-prototype) Starshot product is released, and we can truly evaluate the demand for Lando support.

As a postscript, I want to apologize for changing my position on this. I have NO philosophical or technical objections to supporting Lando, but as the reality of this initiative sinks in, and its scope and reach are defined, and as the necessary maintenance becomes clear...it just doesn't feel quite right at this time.

phenaproxima avatar Jul 24 '24 02:07 phenaproxima