A few more improvements to the README.md #13

rpkoller · 2024-03-19T14:13:49Z

I've already created a draft before (#9) but thanks to my "excellent" git and github skills something went wrong when i tried to sync my branch with the latest changes when randys PR went in. I've simply created a new PR now, that was faster incorporating my suggested changes i've done locally so far.

and again at first thank you for creating that excellent add-on! and as a disclaimer i am not a developer just a regular ddev user who was just used to a composer patches based workflow for testing issues on drupal.org. when i first tried the add-on i ran into a few stepping blocks since it is following a slightly different paradigm (i've expecting the folder structure with web as the docroot and i've installed drush wondering why i haven't had a git status without any changes). one or two weeks ago i've also noticed a support request on the drupal slack by someone trying to get the add-on work (who had basically the same issues like me). so based on my own experience as well as the details i've learned during the conversation on slack i've tried to come up with a few improvements to the README.md:

added a TOC and structured the entire document with headlines
untwine the long codeblock at the beginning and move the different parts into the appropriate sections
tried to make it clear nothing should be installed (required) with composer like for example drush (therefore the warning box to make it extra clear)
added the omit the database container flag since the addon is using SQLite (to save a few megabyte but at the same time to help the user to understand how the add is working and what database is used - which is also relevant in case someone wants to drop the database tables in case one is working on a patch in the context of the drupal installer).

plus i've added the suggestions @rfay made over on the old PR into this one

README.md

T-Fletcher

Looks like #17 was merged in, you may want to rebase this so we don't double up on changes. In particular, @rfay's line about running DDEV with preset flags for overrides, php version etc.

Given my #16 was getting messy, I'm happy to work on this one instead as we're working towards the same goal.

rpkoller · 2024-03-27T15:30:09Z

Looks like #17 was merged in, you may want to rebase this so we don't double up on changes. In particular, @rfay's line about running DDEV with preset flags for overrides, php version etc.

thank you, i've rebased my branch against upstream/main now. i was under the impression since the github ui showed that the branch has no conflicts with the base branch it wouldn't be necessary. but definitely the better and cleaner choice.

Given my #16 was getting messy, I'm happy to work on this one instead as we're working towards the same goal.

i think we shouldnt expand the scope and pack more things into this PR. I would keep this PR about simply restructuring the README to improve hopefully readability and clarity. The PR you have started is more about functionality and that the quickstart guide runs through without any stumbling blocks.

And in regards of the continuing problems #16 and also #20 @simesy tried to fix alongside the changes in the upcoming version 1.23.0 of DDEV https://github.com/ddev/ddev/releases/tag/v1.23.0-alpha1 @rfay suggested in a conversation on slack to do the following:

I think that the 3-4 contributors trying to sort things out in the justafish repo should just sit down and collaborate. Slack channel for that might not be a bad idea. Competing PRs are just going to waste justafish’s time and energy

that was the reason behind simes posting (#22). so you are more than welcome to join the discussion on the newly created channel.

T-Fletcher · 2024-03-27T22:10:04Z

Sounds like a smart approach, I agree with trying to keep functionality vs readability separate but note this can be messy when we are working on the same files via separate PRs.

What's the Slack channel called?

rfay · 2024-03-27T22:26:40Z

See

[NOTICE] Please join #ddev-for-core-dev for collaboration #22

@T-Fletcher

rfay

Thanks for this, it's going to be great.

Minor suggestions. Should go in ASAP.

README.md

rfay · 2024-04-27T19:35:50Z

README.md


 ```
 git clone https://git.drupalcode.org/project/drupal.git drupal
 cd drupal
-ddev config --project-type=drupal10
+ddev config --omit-containers=db --disable-settings-management
 ddev start
 ddev corepack enable


This really shouldn't be needed any more, if we do a ddev config --update. Also, the @simesy #23 handles this right?

Suggested change

ddev corepack enable

ddev exec corepack enable

as i've mentioned on discord. ddev config --omit-containers=db --disable-settings-management is the exact line used on @simesy's PR. the only overlapping line. therefore i've adjusted it here to avoid conflicts and hassle. and i think those two flags are still needed. for one to avoid the db containers plus to disable the settings management. the rest is done bei config --update

in regards your corepack suggestion. all the quickstart commands i havent bothered. all of those are taken care of in simes PR. and that corepack line is obsolete and not necessary anymore. config --update takes care of that.

Oh, that PR removes this line? OK. But at least it could be correct. There isn't a ddev corepack anywhere. All good.

rfay · 2024-04-27T19:37:39Z

README.md

 ddev start
 ddev corepack enable
 ddev get justafish/ddev-drupal-core-dev
 ddev restart
 ddev composer install
+````
+Because `ddev-drupal-core-dev` is using SQLite the database container could be omitted on `ddev config --project-type=drupal10 --omit-containers=db`.


This has already been done above.

Suggested change

Because `ddev-drupal-core-dev` is using SQLite the database container could be omitted on `ddev config --project-type=drupal10 --omit-containers=db`.

In this comment I try to explain "why" the --omit-container flag was actually used.

IMO it would be better to do that in a comment above the command. Here it's out of context.

README.md

rfay

Noticed problem with ddev corepack, which isn't a thing. I don't think we even have to do this when the other PR goes in.

rfay · 2024-04-27T20:33:26Z

README.md


 ```
 git clone https://git.drupalcode.org/project/drupal.git drupal
 cd drupal
-ddev config --project-type=drupal10
+ddev config --omit-containers=db --disable-settings-management
 ddev start
 ddev corepack enable


Suggested change

ddev corepack enable

ddev exec corepack enable

rfay · 2024-04-27T20:34:06Z

README.md

 ddev start
 ddev corepack enable
 ddev get justafish/ddev-drupal-core-dev
 ddev restart
 ddev composer install
+````
+Because `ddev-drupal-core-dev` is using SQLite the database container could be omitted on `ddev config --project-type=drupal10 --omit-containers=db`.


IMO it would be better to do that in a comment above the command. Here it's out of context.

rfay

Thanks, it should be fine! I'd rather have ddev exec corepack enable if it's going to be there at all but we can wait for that.

…duction

Co-authored-by: Randy Fay <[email protected]>

sdc went stable and the given sdc directory contains only a single yaml file. no tests no nothing anymore. Co-authored-by: Randy Fay <[email protected]>

…to the quickstart as a comment

… drupal uninstall

rpkoller marked this pull request as ready for review March 19, 2024 14:15

rpkoller mentioned this pull request Mar 24, 2024

Updated CLI steps to avoid Drupal installation errors #16

Closed

T-Fletcher reviewed Mar 27, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

T-Fletcher reviewed Mar 27, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

T-Fletcher reviewed Mar 27, 2024

View reviewed changes

rpkoller force-pushed the 20240319-rpkoller-readme-improvement branch from 358c6b3 to 93a1cb9 Compare March 27, 2024 14:52

rfay suggested changes Apr 27, 2024

View reviewed changes

rfay approved these changes Apr 27, 2024

View reviewed changes

rpkoller force-pushed the 20240319-rpkoller-readme-improvement branch from c393e34 to 261a374 Compare April 29, 2024 15:02

rpkoller and others added 10 commits April 30, 2024 14:46

Initial draft for the README.md improvements

1a6c78b

add the missing ddev to ddev composer require

bcc57ab

Fix the links in the TOC and add a link to the ddev repo in the intro…

520f50c

…duction

update the flags for ddev config

1c66eee

Rename PHP Unit to PHPUnit

af8ad0b

Co-authored-by: Randy Fay <[email protected]>

More explicit warning NOT to composer require drush

8da53e6

Co-authored-by: Randy Fay <[email protected]>

Some improvement to the drop db sentence

c28ca98

Co-authored-by: Randy Fay <[email protected]>

Update the info for nightwatch

1af7897

Co-authored-by: Randy Fay <[email protected]>

change the sdc example to workspaces

afbf74b

sdc went stable and the given sdc directory contains only a single yaml file. no tests no nothing anymore. Co-authored-by: Randy Fay <[email protected]>

Move the explanation about the database containe omission is moved in…

c26b7e8

…to the quickstart as a comment

rpkoller force-pushed the 20240319-rpkoller-readme-improvement branch from 261a374 to c26b7e8 Compare April 30, 2024 12:46

change the instructions for dropping the db from manually deletion to…

3eec774

… drupal uninstall

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A few more improvements to the README.md #13

A few more improvements to the README.md #13

rpkoller commented Mar 19, 2024 •

edited

Loading

T-Fletcher left a comment

rpkoller commented Mar 27, 2024 •

edited

Loading

T-Fletcher commented Mar 27, 2024

rfay commented Mar 27, 2024

rfay left a comment

rfay Apr 27, 2024

rfay Apr 27, 2024

rpkoller Apr 27, 2024

rpkoller Apr 27, 2024

rfay Apr 27, 2024

rfay Apr 27, 2024

rpkoller Apr 27, 2024

rfay Apr 27, 2024

rfay left a comment

rfay Apr 27, 2024

rfay Apr 27, 2024

rfay left a comment

A few more improvements to the README.md #13

Are you sure you want to change the base?

A few more improvements to the README.md #13

Conversation

rpkoller commented Mar 19, 2024 • edited Loading

T-Fletcher left a comment

Choose a reason for hiding this comment

rpkoller commented Mar 27, 2024 • edited Loading

T-Fletcher commented Mar 27, 2024

rfay commented Mar 27, 2024

rfay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfay left a comment

Choose a reason for hiding this comment

rpkoller commented Mar 19, 2024 •

edited

Loading

rpkoller commented Mar 27, 2024 •

edited

Loading