Consider updating documentation to use drush command

Created on 13 September 2024, 6 months ago

Problem/Motivation

Hello all, feel free to close this issue if this request has already been made (I couldn't find an issue where this was discussed).

Using DDEV, I can't get the commands listed on https://ecaguide.org/library to work. I appreciate that there are a few obvious reasons for this.

1. DDEV instances are run inside a docker container, therefore commands need to be run inside the docker container.
2. For everything else you do with a DDEV instance, you typically do not expect to ssh into your instance and run commands unless something goes wrong.
3. Even when you ssh into your instance the commands are wrong because composer doesn't place recipes into the folder the command expects.

As a result I recommend changing the documentation to use a drush command which solves problem #1 above. While doing this we have the opportunity to fix the relative file location that the composer command currently uses.

So like this. The command on https://ecaguide.org/library/simple/count_user_logins/ would change to:

ddev composer require drupal-eca-recipe/eca_lib_0012
ddev drush cr
ddev drush recipe ../recipes/eca_lib_0012

Oh, I should have added that after running the composer command, a cache clear is needed so that drupal can rerun discovery logic. Perhaps in the use case where you run the composer command without using ddev you can get away of this. But with DDEV, running any command spins up the site, so you need to run the cache clear so that module discovery logic can be rerun and any new modules that the recipe is adding can be found. If you don't run this, the recipe will properly run but error out when a required module (that is definitely, physicially present) isn't found.

Steps to reproduce

Run your recommended recipe script and see if it works for you and your dev setup.

Proposed resolution

Update commands

πŸ› Bug report
Status

Active

Version

2.1

Component

Documentation

Created by

πŸ‡ΊπŸ‡ΈUnited States cosmicdreams Minneapolis/St. Paul

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @cosmicdreams
  • πŸ‡ΊπŸ‡ΈUnited States cosmicdreams Minneapolis/St. Paul
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thanks @cosmicdreams for that proposal, you're absolutely right that using drush is much preferred compared to using the drupal script. And that's true for all environments, not only for ddev.

    The reason why we've done it that way back when we introduced the recipes was, that only Drush 13 supports the recipe command and that version wasn't even released back then.

    That's different now, but I'm not sure how many installations of Drush 12 are still around in the Drupal 10.3 or later environments.

    Maybe we should just extend the documentation and show both options? The composer command stays the same anyway, so it's only about applying the recipe could be done with the script or with Drush. The user can then choose.

    The cash refresh, though, sounds a bit odd. If that's really needed, I'm not sure. If anything, that sounds like a ddev issue and I would keep that out of the ECA documentation.

  • πŸ‡ΊπŸ‡ΈUnited States cosmicdreams Minneapolis/St. Paul

    In my view, the cache clear was needed.

    Give it a go, maybe the issue is just a me thing

  • πŸ‡ΊπŸ‡ΈUnited States cosmicdreams Minneapolis/St. Paul

    In this time when we are in between, with some people using an older version of drush and some using the newest, perhaps we should provide both commands and document what versions they are compatible with.

    I can go through the documentation pages and make the changes myself if that helps.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thanks @cosmicdreams for your suggestions, that all sounds great.

    I can go through the documentation pages and make the changes myself if that helps.

    Fortunately, we don't have to edit all the samples individually. There is a twig template which is used to create all that automatically. You can open an MR here to make that change once, and then we only have to apply that to the documentation, when it's done. You can find that template at eca/modules/development/templates/docs/library.md.twig

  • Pipeline finished with Success
    6 months ago
    Total: 500s
    #285513
  • Status changed to Needs review 6 months ago
  • πŸ‡ΊπŸ‡ΈUnited States cosmicdreams Minneapolis/St. Paul
  • Status changed to Needs work 5 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    This looks like a good starting point. I've left a couple of comments, though.

    I'm wondering, if the required instructions get so verbose, that we may consider a separate chapter for the instructions and simply link to that from each sample page?

  • Pipeline finished with Success
    5 months ago
    Total: 443s
    #287317
  • Pipeline finished with Skipped
    3 months ago
    #362446
  • Pipeline finished with Skipped
    3 months ago
    #362447
  • Status changed to Fixed 3 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • Pipeline finished with Success
    3 months ago
    Total: 631s
    #362455
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • Pipeline finished with Success
    26 days ago
    Total: 234s
    #415004
  • Pipeline finished with Success
    17 days ago
    Total: 469s
    #423192
  • Pipeline finished with Skipped
    11 days ago
    #429031
  • Pipeline finished with Canceled
    11 days ago
    Total: 84s
    #429028
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    For some reason, this got closed without actually merging the MR. Must have been a manual error on my side. I've just merged it now and will update the ECA Guide asap.

Production build 0.71.5 2024