Problem/Motivation

The API documentation for Drupal\Core\Database\Query\SelectInterface::getTables() could be improved, by making some specimen code clearer.

Proposed resolution

Update the specimen code in the documentation block, a simple change:

$fields =& $query->getTables();

The variable should be called $tables, not $fields.

Remaining tasks

Edit core/lib/Drupal/Core/Database/Query/SelectInterface.php
Update the documentation block for the getTables() method.
Write a patch.

User interface changes

none.

API changes

none.

Data model changes

none.

CommentFileSizeAuthor
#7 wrong-var-name-in-sample-code-2939724-7.patch574 bytesryanhayes
#3 wrong-var-name-in-sample-code-2939724-3.patch586 bytesryanhayes

Comments

joachim created an issue. See original summary.

andrewmacpherson’s picture

andrewmacpherson commented
Issue summary: View changes
Issue tags: +SprintWeekend2018

Updating summary, providing novice tasks.
Joachim's link was for 8.2.x - it now points to an 8.5.x API page, for the interface.

We looked at this issue during a FLOSS code contribution workshop today, as part of a web developer apprenticeship program running in the UK. We found our way to the right interface file.

ryanhayes’s picture

ryanhayes commented
Status: Active » Needs review
StatusFileSize
new wrong-var-name-in-sample-code-2939724-3.patch586 bytes
joachim’s picture

joachim commented
Status: Needs review » Reviewed & tested by the community

> Joachim's link was for 8.2.x - it now points to an 8.5.x API page, for the interface.

Sorry about that! Google searches always throw up 8.2.x links for some reason.

Patch looks good to me!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: wrong-var-name-in-sample-code-2939724-3.patch, failed testing. View results

joachim’s picture

joachim commented 
index 6a91e72..e42fcc6 100644
--- a/docroot/lib/Drupal/Core/Database/Query/SelectInterface.php

--- a/docroot/lib/Drupal/Core/Database/Query/SelectInterface.php
+++ b/docroot/lib/Drupal/Core/Database/Query/SelectInterface.php

Ah. The file path's wrong.

(Can I recommend dorgflow for making patches painlessly?)

ryanhayes’s picture

ryanhayes commented
Status: Needs work » Needs review
StatusFileSize
new wrong-var-name-in-sample-code-2939724-7.patch574 bytes

Corrected, although it was pointed out before i uploaded it. I wanted to see what would happen :)

Yeah i'll check out Dorgflow.

Adam_Moulsdale’s picture

Adam_Moulsdale
Primary language English
Location Manchester
commented

I can confirm that the issue explained in #6 has been fixed in the most recent patch.

samvel’s picture

samvel commented

Reviewed and applied patch. Installed clean and correctly.
Good work! RTBC!

samvel’s picture

samvel commented

Let's hide unused patch

recluso’s picture

recluso commented
Status: Needs review » Reviewed & tested by the community

I think Samvel meant to update the status.

samvel’s picture

samvel commented

Yes! i only waiting patch test results :) But they good too!

andrewmacpherson’s picture

andrewmacpherson commented

Awesome - issue .filed to RTBC in under 5 hours!

@SachaPontes and @Adam_Moulsdale all deserve a first-time contribution credit here. After the first patch failed, the session involved a bit of pair-programming, patch-rerolling, git man-page reading, ...

Today's work was part of a first-time contribution workshop, run in Leeds by myself and @rachel_norfolk. The participants were from an apprenticeship scheme, leading to a recognized qualification.

  • Gábor Hojtsy committed 441f198 on 8.5.x 
    Issue #2939724 by ryanhayes, joachim, Samvel, andrewmacpherson, sacha@...

Gábor Hojtsy credited rachel_norfolk.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: wrong variable name in sample code » Wrong variable name in SelecInterface sample code
Status: Reviewed & tested by the community » Fixed

Yay, thanks all!

  • Gábor Hojtsy committed cc31a64 on 8.6.x 
    Issue #2939724 by ryanhayes, joachim, Samvel, andrewmacpherson, sacha@...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 8.6.x-dev » 8.5.x-dev

Status: Fixed » Closed (fixed)

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