Split "This maybe" into "This may be", hyphenated some adjectives, and standardized the naming of modules.

Files: 
CommentFileSizeAuthor
#8 1829366-settings-grammar-3.patch974 bytesgcassie
PASSED: [[SimpleTest]]: [MySQL] 46,601 pass(es).
[ View ]
#5 1829366-settings-grammar-2.patch1.66 KBgcassie
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1829366-settings-grammar-2_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 1829366-settings-grammar.patch3.61 KBgcassie
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1829366-settings-grammar.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new3.61 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1829366-settings-grammar.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

patch attached

Status:Needs review» Needs work

The last submitted patch, 1829366-settings-grammar.patch, failed testing.

I have objection to these two

+ * By default, Drupal configuration files are stored in a randomly-named
+ * of heavily-visited sites and may also provide other site caching,

The added hyphen is unnatural and unneeded.

RE #3 - actually I think those two changes are correct by English grammar/punctuation rules.

The changes in the patch look fine to me. But please take the chunk out that is not default.settings.php (we try to keep issues to one topic only). Thanks!

Status:Needs work» Needs review
StatusFileSize
new1.66 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1829366-settings-grammar-2_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Sorry, the SessionTest stuff snuck in. New patch for default.settings.php only attached.

I double-checked the hyphens and since the words in question here are adverbs, they are not strictly needed. The new patch omits them.

Assigned:Unassigned» jhodgdon
Status:Needs review» Reviewed & tested by the community
Issue tags:+needs backport to D7

Thanks - this patch looks fine! I'll get it committed next week (there are a lot of sprints happening this weekend for BADCamp and committing patches ties up the test bot, so postponing a few days). This patch will most likely still apply to 7.x too; if not, after commit to 8.x someone can backport.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1829366-settings-grammar-2.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new974 bytes
PASSED: [[SimpleTest]]: [MySQL] 46,601 pass(es).
[ View ]

Looks like the Ban logic was removed from default.settings.php .

Status:Needs review» Reviewed & tested by the community

Thanks!

Assigned:jhodgdon» Unassigned
Status:Reviewed & tested by the community» Fixed

Thanks! Sorry for the delay (there were sprints going on and I was avoiding commits for a while and then got busy) -- committed to 8.x and 7.x.

gcassie: is that your first core patch? Congrats!

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