Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
ReplicaDatabaseIgnoreSubscriber
has the following documentation:
// ... Code that wants to disable the replica server should
// use the db_set_ignore_replica() function to set
// $_SESSION['ignore_replica_server'] to the timestamp after which the replica
// can be re-enabled.
There is no function db_set_ignore_replica()
, though. The actual function is called db_ignore_replica()
.
Proposed resolution
Replace db_set_ignore_replica()
with db_ignore_replica()
.
For extra credit reformat the code block so that every line breaks at 80 characters.
Remaining tasks
After this issue is committed, re-open #2488440: Reformat comments to 80 characters in ReplicaDatabaseIgnoreSubscriber::checkReplicaServer() comments.
User interface changes
None
API changes
None
Beta phase evaluation
Issue category | Bug: see #9 |
---|---|
Issue priority | Minor: see #9 |
Unfrozen changes | Unfrozen because it only changes documentation. |
Comment | File | Size | Author |
---|---|---|---|
#25 | 2486549-nr-bot.txt | 133 bytes | needs-review-queue-bot |
#11 | 2486549-9.patch | 942 bytes | mgifford |
#9 | 2486549-9.patch | 942 bytes | benjifisher |
#1 | 2486549-1.patch | 2 KB | joshi.rohit100 |
Comments
Comment #1
joshi.rohit100Comment #2
tstoecklerAwesome, thanks!
Comment #3
xjmSo actually (contrary to the suggestion in summary), we should not do cleanups like rewrapping comments to meet our 80-character limit in a patch that's not specifically for that, as it adds a lot of out-of-scope things to review that are not related to the bugfix in the patch. For more information, see: http://webchick.net/please-stop-eating-baby-kittens
Let's update this patch to only make the change in the title. Thanks!
Comment #4
tstoecklerNope. This is a 2Kb patch. Let's stay realistic please. In the time you wrote that comment you could have already have reviewed the patch.
Comment #5
xjm@tstoeckler, of course I could. (Edit: actually I did.) But @joshi.rohit100 is a new contributor who has been submitting a lot of great work, so part of the goal is to help communicate best practices for that work. Thanks!
Comment #6
tstoecklerWhat you call "communicating" is what I would call "scaring people off". Anyway, you're the boss. Unsubscribe.
Comment #7
benjifisherI added #2488440: Reformat comments to 80 characters in ReplicaDatabaseIgnoreSubscriber::checkReplicaServer() comments as a follow-up to deal with wrapping the comments to 80 characters. I also edited the summary, crossing out the suggestion to do that as part of this issue. (I did not completely delete it, since that would make earlier comments confusing.)
I think that if we stick to the change recommended in #3, then this still deserves the Novice tag.
Comment #8
jcandan CreditAttribution: jcandan as a volunteer commentedI am removing the Novice tag from this issue because it's lacking consensus on the issue of splitting it.
I’m using this documentation as a source: https://www.drupal.org/core-mentoring/novice-tasks#avoid
Comment #9
benjifisher@jcandan: Not every disagreement counts as lack of consensus. IMHO, if two people disagree and one of them decides to stop following the issue, then the next step is clear.
The attached patch fixes just the problem described in the title, as requested in #3.
I reviewed https://www.drupal.org/core/issue-category and decided that this issue is a bug, not a task: "Incorrect or misleading documentation" is one of the examples of a bug.
I reviewed https://www.drupal.org/core/issue-priority and decided to classify this as "minor". It is pretty close to the example of a minor bug: "An incorrect class reference only in a comment."
Comment #10
benjifisherComment #11
mgiffordRe-uploading patch for the bots.
Comment #18
andypostThis could be closed in favour of #2286235: Deprecate db_ignore_replica() and convert it to service
Just needs to migrate commit credits
Comment #25
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.