Make it extremely easy to boot up Starshot with LANDO #55

MatthieuScarset · 2024-05-08T19:18:43Z

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

README.md

.lando.yml

phenaproxima · 2024-05-09T02:33:10Z

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?

MatthieuScarset · 2024-05-10T19:13:37Z

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 🤷

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

Taiger · 2024-05-10T21:51:33Z

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!

AlexSkrypnyk · 2024-05-10T23:42:34Z

@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.

Taiger · 2024-05-11T16:49:53Z

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.

phenaproxima · 2024-05-13T14:21:48Z

@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

A couple of very minor changes, and the merge conflicts need to be resolved.

Ideally we would also inject the DRUSH_OPTIONS_URI environment variable into the app server container always, so that you never really have to think about passing --uri.

.lando.yml

phenaproxima · 2024-05-13T14:28:22Z

.lando.yml

+      echo ""
+      echo "🚀 Board the spaceship!"
+      echo "👇 Click on the link below to login 👇"
+      drush uli -l 'https://starshot.lndo.site'


So, rather than have to pass -l or --uri, could we do a similar thing to what we're doing with DDEV and inject a DRUSH_OPTIONS_URI environment variable into the app server that has the same value as https://$LANDO_DOMAIN? That way, Drush will always have the correct URI when run within the app server container and we wouldn't have to remember to pass it.

Yes we can pass the DRUSH_OPTIONS_URI in Lando like this:

services: appserver: overrides: environment: DRUSH_OPTIONS_URI: "http://mysite.lndo.site"

But it does not work because we have a drush.yml file committed with a localhost value - and it takes precedence over the environment variables (see previous comment).

@phenaproxima do we need the drush.yml file committed?

If no, then we can use the env var for Drush URI.

If yes, should I add the echo to rewrite it again?

README.md

gitressa · 2024-05-13T17:43:21Z

@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.

Co-authored-by: Adam <adam@phenaproxima.net>

AlexSkrypnyk · 2024-05-13T20:39:52Z

@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

lauriii · 2024-05-23T13:24:19Z

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.

MatthieuScarset · 2024-05-24T04:08:07Z

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:
#55 (comment)

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

phenaproxima · 2024-05-25T02:04:36Z

@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.

MatthieuScarset · 2024-05-25T14:34:27Z

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

Manish-Sharma1995 · 2024-05-29T09:12:56Z

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

Make it extremely easy to boot up Starshot with LANDO

ec0c4ac

phenaproxima reviewed May 8, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

phenaproxima reviewed May 8, 2024

View reviewed changes

.lando.yml Outdated Show resolved Hide resolved

MatthieuScarset added 2 commits May 10, 2024 21:05

Better tooling

4677363

Update README

524ccfa

phenaproxima requested changes May 13, 2024

View reviewed changes

MatthieuScarset and others added 3 commits May 13, 2024 21:13

Move lando command onto their own line

08816bf

Co-authored-by: Adam <adam@phenaproxima.net>

Merge phenaproxima:main

ffab37e

Merge branch 'main' into main

b8c3fad

MatthieuScarset requested a review from phenaproxima May 16, 2024 08:10

Update greeting message after drupal install

729fa40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make it extremely easy to boot up Starshot with LANDO #55

Make it extremely easy to boot up Starshot with LANDO #55

MatthieuScarset commented May 8, 2024

phenaproxima commented May 9, 2024

MatthieuScarset commented May 10, 2024

Taiger commented May 10, 2024 •

edited

AlexSkrypnyk commented May 10, 2024 •

edited

Taiger commented May 11, 2024 •

edited

phenaproxima commented May 13, 2024 •

edited

phenaproxima left a comment

phenaproxima May 13, 2024

MatthieuScarset May 13, 2024

MatthieuScarset May 13, 2024

gitressa commented May 13, 2024 •

edited

AlexSkrypnyk commented May 13, 2024

lauriii commented May 23, 2024

MatthieuScarset commented May 24, 2024 •

edited

phenaproxima commented May 25, 2024

MatthieuScarset commented May 25, 2024

Manish-Sharma1995 commented May 29, 2024

Make it extremely easy to boot up Starshot with LANDO #55

Are you sure you want to change the base?

Make it extremely easy to boot up Starshot with LANDO #55

Conversation

MatthieuScarset commented May 8, 2024

phenaproxima commented May 9, 2024

MatthieuScarset commented May 10, 2024

Taiger commented May 10, 2024 • edited

AlexSkrypnyk commented May 10, 2024 • edited

Taiger commented May 11, 2024 • edited

phenaproxima commented May 13, 2024 • edited

phenaproxima left a comment

Choose a reason for hiding this comment

phenaproxima May 13, 2024

Choose a reason for hiding this comment

MatthieuScarset May 13, 2024

Choose a reason for hiding this comment

MatthieuScarset May 13, 2024

Choose a reason for hiding this comment

gitressa commented May 13, 2024 • edited

AlexSkrypnyk commented May 13, 2024

lauriii commented May 23, 2024

MatthieuScarset commented May 24, 2024 • edited

phenaproxima commented May 25, 2024

MatthieuScarset commented May 25, 2024

Manish-Sharma1995 commented May 29, 2024

Taiger commented May 10, 2024 •

edited

AlexSkrypnyk commented May 10, 2024 •

edited

Taiger commented May 11, 2024 •

edited

phenaproxima commented May 13, 2024 •

edited

gitressa commented May 13, 2024 •

edited

MatthieuScarset commented May 24, 2024 •

edited