Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The install.txt file needs to be reviewed:
- Check whether all links to the online documentation are still correct
- Check whether they use https.
- Correct drupal-x.y to drupal-x.y.z
Compact Beta evaluation: This is a documentation-only change to INSTALL.txt only. No disruption. Not frozen.
Comment | File | Size | Author |
---|---|---|---|
#31 | interdiff-2371587-28-31.txt | 643 bytes | fullerja |
#31 | review_install_txt-2371587-31.patch | 8.66 KB | fullerja |
Comments
Comment #1
batigolixFailed attempt to improve the install.txt file:
#1942316: Rewrite INSTALL.txt with links to full docs on drupal.org
Comment #2
batigolixComment #3
jhodgdonComment #4
shrijata CreditAttribution: shrijata commentedWorking on this.
Comment #5
shrijata CreditAttribution: shrijata commentedThe 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.
Comment #6
shrijata CreditAttribution: shrijata commentedComment #7
David Hernández CreditAttribution: David Hernández commentedHello!
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 #.
Comment #8
FreekyMage CreditAttribution: FreekyMage commentedLinks 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.
Comment #9
fullerja CreditAttribution: fullerja commentedAdded 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.
Comment #10
fullerja CreditAttribution: fullerja commentedComment #11
jhodgdonThanks 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?
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)
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.
Comment #12
fullerja CreditAttribution: fullerja commentedThanks 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.
Comment #15
jhodgdonSorry, 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:
Comment #16
fullerja CreditAttribution: fullerja commentedFixed, thanks for the guidance.
Comment #17
fullerja CreditAttribution: fullerja commentedComment #18
jhodgdonThanks! 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.
Comment #19
fullerja CreditAttribution: fullerja commentedI 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.
vs.
Comment #20
Prashant.cPatch #19 applied successfully.
Submitting the patch with minor indentation and changing link for multi-site configuration documentation.
Comment #21
jhodgdonPLEASE make interdiff files when you submit new patches on existing issues!!!!!!!
This latest patch still has lines longer than 80 characters, such as:
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. :)
Comment #22
fullerja CreditAttribution: fullerja commentedAdded the link and spacing fix from #20. Good catch on the redirect. Interdiffs from #19 and #20 included.
Comment #23
jhodgdonThis fix is not right:
The indentation of these lines was meant to indicate that these files are inside sites/sub.example.com.
Comment #24
fullerja CreditAttribution: fullerja commentedAdded full directory paths.
Thanks again for all the help getting is reviewed. I really appreciate it.
Comment #25
jhodgdonCan you please just not change that section of the patch? The indentation was fine as it was.
Comment #26
fullerja CreditAttribution: fullerja commentedComment #27
jhodgdonHi! 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:
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.
Comment #28
fullerja CreditAttribution: fullerja commented@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'.
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.
Comment #29
fullerja CreditAttribution: fullerja commentedComment #30
jhodgdonThis 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):
There is no scripts directory in Drupal 8. It's core/scripts now.
I think if we fix that, we're good to go!
Comment #31
fullerja CreditAttribution: fullerja commentedMade the change for the scripts directory.
Comment #32
jhodgdonExcellent, thanks! I think this looks good now. Adding beta eval to issue summary.
Comment #33
alexpottCommitted 4369860 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation to issue summary.
Comment #35
David_Rothstein CreditAttribution: David_Rothstein commentedI 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.