Problem/Motivation

From 8.1.x in case we clone Drupal using git. The installation procedure requires to run composer install to download all the dependencies. Currently INSTALL.txt has this step missing. Mostly new developers would refer to INSTALL.txt to install Drupal so adding this information would be helpful.

Proposed resolution

* Add step to run composer install in INSTALL.txt

Remaining tasks

* Write Patch to fix the documentation

User interface changes

*None

API changes

*None

Data model changes

CommentFileSizeAuthor
#21 interdiff.2762173.20.21.txt773 bytesYesCT
#21 WonderWoman2-2762173-21.patch1007 bytesYesCT
#20 interdiff-2762173-18-20.txt636 bytesmegansanicki
#20 WonderWoman2-2762173-20.patch1006 bytesmegansanicki
#18 interdiff-2762173-13-18.txt1.16 KBmegansanicki
#18 2762173-WonderWoman.patch1002 bytesmegansanicki
#14 gitinstructions_2762173_10_14.txt1.28 KBartinst4
#13 gitinstructions_2762173_13.patch967 bytesartinst4
#10 gitinstructions_2762173_10.patch912 bytesartinst4
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

a_thakur created an issue. See original summary.

vermauv’s picture

vermauv CreditAttribution: vermauv at Srijan | A Material+ Company commented
Assigned: Unassigned » vermauv
cilefen’s picture

cilefen CreditAttribution: cilefen commented

This seems a possible duplicate of #2737773: Proper way to install Drupal, missing vendor folders, example.gitignore.

vermauv’s picture

vermauv CreditAttribution: vermauv at Srijan | A Material+ Company commented
Issue summary: View changes
a_thakur’s picture

a_thakur CreditAttribution: a_thakur as a volunteer and at Srijan | A Material+ Company commented
Issue summary: View changes

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

artinst4’s picture

artinst4 CreditAttribution: artinst4 as a volunteer commented
Title: Steps to execute composer install when Drupal is downloaded using git » Add "composer install" step to install.txt file for when Drupal is downloaded using git
Assigned: vermauv » Unassigned
Issue tags: +MWDS2016
Related issues: +#2737773: Proper way to install Drupal, missing vendor folders, example.gitignore

@vermauv, thanks for your interest in this issue. Unassigning it since it seems that you aren't currently working on it.

@cilefen #3, I took a look at the other other issue. It seems to be talking mostly about a problem with .gitignore and a general procedure for install. A much larger scope.

I am using this issue specifically for adding the "composer install" step to the install.txt file.

Next, I will make a patch to add the step to the install.txt file.

artinst4’s picture

artinst4 CreditAttribution: artinst4 as a volunteer commented
Category: Task » Bug report

Based on the definition described in documentation, the category is "bug", so I've gone ahead and changed that.
See the criteria:

  • Incorrect or misleading documentation

https://www.drupal.org/core/issue-category#bug

artinst4’s picture

artinst4 CreditAttribution: artinst4 as a volunteer commented
Version: 8.3.x-dev » 8.1.x-dev

Based on the documentation at https://www.drupal.org/core/d8-allowed-changes#disruption, this is also "non-disruptive". It involves a small change to an isolated text file.

Since this is a non-disruptive bug, the patch can be applied to 8.1.x. See here: https://www.drupal.org/core/d8-allowed-changes#patch.

artinst4’s picture

artinst4 CreditAttribution: artinst4 as a volunteer commented
FileSize
gitinstructions_2762173_10.patch912 bytes

Here's a patch that reflects the changes to install.txt mentioned in comment #9.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe at Kosada commented
Status: Active » Needs work

Thank you for the patch, @artinst4! Remember to set your issue status to "Needs Review", which will let other people know to review.

  1. +++ b/core/INSTALL.txt
@@ -90,6 +90,15 @@ INSTALLATION
+   You can also download the latest version of Drupal using git and set up a repository
+   by following the instructions at https://www.drupal.org/project/drupal/git-instructions for "Setting up repository for the first time".
+
+   Once you've downloaded Drupal successfully, run the following commands to install using "Composer",

    These lines should for the most part adhere to the 80-character line limit.

    I looked at INSTALL.txt, and this document complies with the 80-character line limit except when a URL or code/command snippet is used such as on line 92. 

  2. +++ b/core/INSTALL.txt
@@ -90,6 +90,15 @@ INSTALLATION
+     composer install
+     composer update

    I do not think this is accurate because an user should only need to run composer install.

    composer update is used to update dependencies. This would cause the Drupal installation to potentially update its Symfony dependencies in composer.json. composer install will install the version locked dependencies in the included composer.lock file.

  3. This should also have a patch for Drupal 8.2.x.
mathieso’s picture

mathieso CreditAttribution: mathieso as a volunteer commented

Nice work. Some suggestions.

1. Perhaps add text telling people why they need to run composer. Something like:

Drupal takes advantage of code from other open source projects. That code is not included with Drupal, and must be downloaded separately. You can use the command line tool Composer to do that for you.

2. Mention that the command "composer install" is run at the command line.

3. Perhaps remove the quotes from this:

install using "Composer",

To:

install using Composer,

It doesn't matter much, but the quotes will annoy some people who are strict about punctuation.

artinst4’s picture

artinst4 CreditAttribution: artinst4 as a volunteer commented
Version: 8.1.x-dev » 8.2.x-dev
Status: Needs work » Needs review
FileSize
gitinstructions_2762173_13.patch967 bytes

Thanks @mathieso and @mradcliffe. Here is an updated patch integrating some of your suggested edits. I am changing it to version 8.2.x, since it is also the environment I tested in.

artinst4’s picture

artinst4 CreditAttribution: artinst4 as a volunteer commented
FileSize
gitinstructions_2762173_10_14.txt1.28 KB

Here is the interdiff file for the recent change as well.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe at Kosada commented
Status: Needs review » Reviewed & tested by the community

Awesome. The text and language is consistent with the rest of INSTALL.txt with the patch in #13.

Thank you for posting the interdiff.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe at Kosada commented

We also need the patches for 8.1.x and 8.3.x, @artinst4.

megansanicki’s picture

megansanicki CreditAttribution: megansanicki at Drupal Association commented
Assigned: Unassigned » megansanicki
Status: Reviewed & tested by the community » Needs work

I tested this myself as a new contributor and ran into two problems.

1) Composer install did not work that is because composer needed to be installed globally and that was not mentioned in the instructions. https://getcomposer.org/doc/00-intro.md#globally

2) When I went to install composer, the instructions didn't tell me to be in the drupal directory so I it would not install.

I will do a reroll with these adjustments

megansanicki’s picture

megansanicki CreditAttribution: megansanicki at Drupal Association commented
Status: Needs work » Needs review
FileSize
2762173-WonderWoman.patch1002 bytes
interdiff-2762173-13-18.txt1.16 KB

Here's my patch. I made the two changes mentioned above and also fixed some line wrapping.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin as a volunteer commented
Status: Needs review » Needs work 
+++ b/core/INSTALL.txt
@@ -90,6 +90,18 @@ INSTALLATION
+   Once you've downloaded Drupal successfully, you must install Composer globally

Looks like it's 81 characters

megansanicki’s picture

megansanicki CreditAttribution: megansanicki at Drupal Association commented
Status: Needs work » Needs review
FileSize
WonderWoman2-2762173-20.patch1006 bytes
interdiff-2762173-18-20.txt636 bytes

Attached is my patch that corrects the text wrapping issue.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
FileSize
WonderWoman2-2762173-21.patch1007 bytes
interdiff.2762173.20.21.txt773 bytes

@megansanicki Thanks for sticking with this issue and addressing @pwolanin's feedback, and making the interdiff. interdiffs are really nice for reviewers.

  1. +++ b/core/INSTALL.txt
@@ -90,6 +90,19 @@ INSTALLATION
+   https://www.drupal.org/project/drupal/git-instructions
+   for "Setting up repository for the first time".

    this can be wrapped closer to 80 chars.

    https://www.drupal.org/node/1354#drupal

    "must wrap as close to 80 characters"

    and checked, the phrase in quotes, Setting up repository for the first time, is the exact header where the instructions are at the url. 

  2. +++ b/core/INSTALL.txt
@@ -90,6 +90,19 @@ INSTALLATION
+   Once you've downloaded Drupal successfully, you must install Composer

    "you've" is a contraction which can be a bit difficult for non-first-english speakers.

    Sorry, can't find the standard which says something about contractions.

    ---

    must -> may
    since people *could* not install it globally, but it's still a good recommendation as it's the common way. 

  3. +++ b/core/INSTALL.txt
@@ -90,6 +90,19 @@ INSTALLATION
+   https://getcomposer.org/doc/00-intro.md#globally

    Also I checked this url and the other, and they go to working correct places.

I one concern, which is: in the section about the tar, says to move the files (the mv command is not the best though), but I think it's ok to not have a section here. Adding the note about composer install is the really important thing and really needed.

Keeping at needs review for someone to check my changes. I think this is ready, if someone else does.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Version: 8.2.x-dev » 8.1.x-dev

I think this can go into 8.1.x and 8.2.x and 8.3.x (non-distruptive docs fix https://www.drupal.org/core/d8-allowed-changes#patch),
it applies to all.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Status: Needs review » Reviewed & tested by the community

had @pwolanin check this in person at the sprint. he says rtbc. :)

webchick credited mlhess.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented

Saving credit.

  • webchick committed 4dcb2b1 on 8.1.x 
    Issue #2762173 by megansanicki, artinst4, YesCT, mradcliffe, a_thakur,...

  • webchick committed 0e0608d on 8.3.x 
    Issue #2762173 by megansanicki, artinst4, YesCT, mradcliffe, a_thakur,...

  • webchick committed ecd02ec on 8.2.x 
    Issue #2762173 by megansanicki, artinst4, YesCT, mradcliffe, a_thakur,...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Ok, committed and pushed to all three branches! WOOT! GO MEGAN!!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.