Problem/Motivation

Updater::update calls Updater::makeBackup() function with 2 arguments but it requires three.

Proposed resolution

Add one argument

Remaining tasks

User interface changes

No

API changes

No

Original report by @droplet

5. Updater::update calls Updater::makeBackup() function with 2 arguments but it requires three.

#1423460: [META] List of bugs and typos in Drupal core found by Spleshka

CommentFileSizeAuthor
#6 drupal-missed_arg-1476804-6.patch561 bytesLordshipkir
updater.patch454 bytesdroplet

Comments

oriol_e9g’s picture

oriol_e9g
Primary language Catalan
Location Barcelona
commented
Status: Needs review » Reviewed & tested by the community

Go, but makeBackup it's a not implemented function.

dries’s picture

dries commented

Should we have a test for this?

droplet’s picture

droplet commented

I surprised this is an empty function

  /**
   * Perform a backup.
   *
   * @todo Not implemented.
   */
  public function makeBackup(&$filetransfer, $from, $to) {
  }
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

I'd like to understand a bit more why we have the stub function in core at all before committing this one.

abghosh82’s picture

abghosh82 commented

Not sure if this is a valid issue still, there is no file as "updater.inc" within "core/includes".

Lordshipkir’s picture

Lordshipkir commented
Issue summary: View changes
Issue tags: +dcdn-sprint
StatusFileSize
new drupal-missed_arg-1476804-6.patch561 bytes

The file updater.inc was in directory /core/lib/Drupal/Core/Updater/.

Lordshipkir’s picture

Lordshipkir commented
Issue summary: View changes

Update issue summary.

Lordshipkir’s picture

Lordshipkir commented
Issue summary: View changes
yesct’s picture

yesct commented
Issue summary: View changes
Issue tags: +Needs tests

I think we do need a test. if this is a bug.

updating the issue summary with contributor task docs.

andypost’s picture

andypost
he/him
Primary language Russian
commented

There's nothing to test because makeBackup() has no implementation for a long time Drupal 7

I think we can just add a unittest of the Updater class

znerol’s picture

znerol commented
Status: Needs review » Closed (duplicate)

This has been fixed in #2300469: makeBackup() is called without a to parameter in Drupal\Core\Updater\Updater.

yesct’s picture

yesct commented

but there were no tests added in that issue.

znerol’s picture

znerol commented

The other issue summary says:

(Can't test it currently as the makeBackup() is not implemented yet.)

Not sure whether this is true. I suggest opening up a new follow-up of the other issue (because its the one which was committed).