Support from Acquia helps fund testing for Drupal Acquia logo

Comments

batigolix’s picture

batigolix’s picture

Issue summary: View changes
jhodgdon’s picture

Component: install system » documentation
shrijata’s picture

Assigned: Unassigned » shrijata
Issue tags: +#SprintWeekend2015

Working on this.

shrijata’s picture

FileSize
6.25 KB

The links given in the documentation are correct and made changed few links to https.
To Do-A reference to drush needs to be added in the documentation.

shrijata’s picture

Assigned: shrijata » Unassigned
Status: Active » Needs review
David Hernández’s picture

Issue tags: -#SprintWeekend2015 +SprintWeekend2015

Hello!

Thank you for working on this issue!

We should all try and use the same sprint tag. According to https://groups.drupal.org/node/447258 it should be SprintWeekend2015 with no #.

FreekyMage’s picture

Status: Needs review » Needs work

Links that were changed in patch were all checked.
These redirect back to http:// , so not sure if they need to change

These can be changed to https://

Setting this to "needs work" to adjust above if needed and because drush part should still be added.

fullerja’s picture

Added the https for percona.com. I left nginx and postgresql as https because they can be accessed that way and if they switch to actually using https, we will be ready. I also added information about drush to the More Information section.

fullerja’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the patches!

I think there are still some things to be fixed in the latest patch:

a) wget https://drupal.org/files/projects/drupal-x.y.tar.gz

We need this to be x.y.z for 8.0.0 etc., right? That would need to be done throughout the INSTALL.txt file.

b) Why are we changing this to https?

-   website (e.g., http://www.example.com).
+   website (e.g., https://www.example.com).

I think our documentation policy in general is to use http://www.example.com for "some example web site URL".

c) https://drupal.org This should always be www.drupal.org now.

d)

+  Drush is a command line shell and Unix scripting interface for Drupal.
+  Drush core ships with lots of useful commands for interacting with code like
+  modules/themes/profiles. Similarly, it runs update.php, executes sql queries
+  and DB migrations, and misc utilities like run cron or clear cache.
+  See https://github.com/drush-ops/drush.

This is not great writing... Can we clean it up a bit and make it more prose-like? For instance don't say "like modules/themes/profiles", say "like modules, themes, and profiles". Also... it's just very informal writing ... things like "ships with lots of useful commands". And it needs proofreading, like serial commas and acronyms like SQL need to be all caps. Don't use "misc", write out the words. Etc. Basicaly it needs to be rewritten. Or removed. I'm not sure it belongs in install.txt at all come to think of it.

Yeah, let's just take it out of there. If the README doesn't have info about Drush, make an issue to add it there, but I don't think it belongs in INSTALL.txt, which is about installing.

e) Actually, all of that More Information section should be removed. None of it is relevant to installation. Let's get rid of all of it or move it to README, not make it worse by adding more to it.

fullerja’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
6.48 KB
6.48 KB

Thanks for the view, made the noted changes. I removed the entire More Information section, I agree that it doesn't help a user install Drupal.

Status: Needs review » Needs work

The last submitted patch, 12: review_install_txt-2371587-12.patch, failed testing.

jhodgdon’s picture

Sorry, I guess I wasn't clear. I meant that the links to drupal.org should all start with https://www.drupal.org. THey should include the www, but they still should be full URLs starting with https. So this is not quite right:

-   You can obtain the latest Drupal release from http://drupal.org -- the files
+   You can obtain the latest Drupal release from www.drupal.org -- the files
fullerja’s picture

Fixed, thanks for the guidance.

fullerja’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Thanks! Looking better.

So some of the changes in this latest patch have made the lines go over 80 characters. They need to be re-wrapped.

Also there are still a couple of drupal.org links that are not starting with https://www.drupal.org.

fullerja’s picture

Status: Needs work » Needs review
FileSize
8.78 KB

I left references to 'Drupal.org online documentation' alone because it seemed like the phrase is being used as a noun rather than a link.

To Do: determine if sentences that end with a link get a 'closing' period (see below). I couldn't find an answer in the Documentation documentation. Seems like they do not get the period, so I am going to put this to Needs Review.

For detailed instructions, visit
https://www.drupal.org/documentation/multilingual

vs.

For detailed instructions, visit
https://www.drupal.org/documentation/multilingual.
Prashant.c’s picture

Patch #19 applied successfully.
Submitting the patch with minor indentation and changing link for multi-site configuration documentation.

jhodgdon’s picture

Status: Needs review » Needs work

PLEASE make interdiff files when you submit new patches on existing issues!!!!!!!

This latest patch still has lines longer than 80 characters, such as:

+NOTE: for more information about multiple virtual hosts or the configuration settings, visit 
+https://www.drupal.org/documentation/install/multi-site

Probably that line shouldn't start with NOTE anyway, to be consistent with other sections?

RE #19: I agree that if a link ends a sentence it is probably best to leave out the . because it makes the link harder to copy/paste. Also, yes, "Drupal.org online documentation" should not say https://www. :)

fullerja’s picture

Status: Needs work » Needs review
FileSize
9.06 KB
955 bytes
863 bytes

Added the link and spacing fix from #20. Good catch on the redirect. Interdiffs from #19 and #20 included.

jhodgdon’s picture

Status: Needs review » Needs work

This fix is not right:

 accessible to other sites, the setup would look like this:
 
   sites/sub.example.com/
-    settings.php
-    themes/custom_theme
-    modules/custom_module
+  settings.php
+  themes/custom_theme
+  modules/custom_module

The indentation of these lines was meant to indicate that these files are inside sites/sub.example.com.

fullerja’s picture

Status: Needs work » Needs review
FileSize
9.12 KB
650 bytes

Added full directory paths.

Thanks again for all the help getting is reviewed. I really appreciate it.

jhodgdon’s picture

Status: Needs review » Needs work

Can you please just not change that section of the patch? The indentation was fine as it was.

fullerja’s picture

Status: Needs work » Needs review
FileSize
656 bytes
8.87 KB
jhodgdon’s picture

Status: Needs review » Needs work

Hi! Sorry this took so long to review, I've been really busy... thanks for the new patch!

So I still have a few questions:

a) I do not know why we are changing all these URLs to use https when they don't actually use that. For instance, if I go to http://httpd.apache.org/ I am not redirected to https. Let's please only make necessary changes to these README files. For instance, http://drupal.org/(whatever) is now being redireted to https://www.drupal.org(whatever), so we should change those URLs, but in the other cases in this file I think the changes are either wrong or unnecessary. They make the patch bigger and harder to review.

b) Something else I just noticed:

+     mv drupal-x.y.z/* drupal-x.y.z/.htaccess /path/to/your/installation

There are actually a lot of other files in Drupal 8 whose names start with . and these will be missed by this command. This is a problem.

c) And another thing: we need to check the links. For instance, for "creating a subtheme" we should be linking to https://www.drupal.org/node/2165673 for Drupal 8, not the link that is there.

fullerja’s picture

FileSize
8.57 KB
8.57 KB

@jhodgdon: Thanks for working with me to get this patch into good shape, I really appreciate the extra effort you are putting into this.

Links to Drupal Requirements
I went through all the links for apache, mysql, nginx, etc link and switched the non-redirected ones back to http. I also removed and 'www' if it redirects to without.

Moving the hidden files
I looked at a couple options, the one I included the patch seemed like the best. I also looked at mv /.*, but that produces notices which may make a newer user nervous. This command is a little monolithic and overflows the 80 character line, other options I considered were splitting the hidden files into its own 'mv'.

mv drupal-x.y.z/* /path/to/your/installation
mv drupal-x.y.z/.htaccess drupal-x.y.z/.csslintrc drupal-x.y.z/.editorconfig drupal-x.y.z/.eslintignore drupal-x.y.z/.eslintrc /path/to/your/installation

or splitting it up to keep much closer to 80 characters per line, but that ends up making each hidden file its own command. I decided to leave it as a single command because that was the previous state. I would appreciate any input.

Module and Theme List links
Module and theme list pages redirect, the URLs are updated in this patch.

Extra notes
https://www.drupal.org/node/157602 - Could use some updates, but I think that should be a separate issue.

fullerja’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

This is looking really good -- thanks for all the revisions!

I went carefully through every URL in the patch and they're all good.

I think (unfortunately) that you did the right thing on that mv command line :( it's sure messy.

I did notice one more thing (and sorry for not noticing it before):

    More information about cron maintenance tasks is available at
-   http://drupal.org/cron, and sample cron shell scripts can be found in the
-   scripts/ directory. (Note that these scripts must be customized like the
+   https://www.drupal.org/cron, and sample cron shell scripts can be found in
+   the scripts/ directory. (Note that these scripts must be customized like the
    above example, to add your site-specific cron key and domain name.)

There is no scripts directory in Drupal 8. It's core/scripts now.

I think if we fix that, we're good to go!

fullerja’s picture

Status: Needs work » Needs review
FileSize
8.66 KB
643 bytes

Made the change for the scripts directory.

jhodgdon’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Excellent, thanks! I think this looks good now. Adding beta eval to issue summary.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 4369860 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to issue summary.

  • alexpott committed 4369860 on 8.0.x
    Issue #2371587 by fullerja, shrijata, Prashant.c: Review install.txt
    
David_Rothstein’s picture

I think it's important to have those security-related links somewhere in the codebase (if not in INSTALL.txt, then somewhere else). Removing them completely seems like a problem. I filed #2426901: Add back various links (especially security-related links) to README.txt or INSTALL.txt to add them back.

Status: Fixed » Closed (fixed)

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