In file: /core/modules/views/src/ViewExecutable.php

For the function setAjaxEnabled comment block @param documented wrong argument . And a little typo in description.

CommentFileSizeAuthor
#6 2603348-2.patch727 bytesrakesh.gectcr
#2 2603348-1.patch788 bytesrakesh.gectcr
wrong-param-typo-doc-block-setAjaxEnabled-1.patch788 bytesrakesh.gectcr

Comments

rakesh.gectcr created an issue. See original summary.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
StatusFileSize
new 2603348-1.patch788 bytes
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented

Here is the changes 

@@ -604,10 +604,13 @@ public function usePager() {
   /**
    * Sets whether or not AJAX should be used.
    *
-   * If AJAX is used, paging, tablesorting and exposed filters will be fetched
+   * If AJAX is used, paging, table sorting and exposed filters will be fetched
    * via an AJAX call rather than a page refresh.
    *
-   * @param bool $use_ajax
+   * @param bool $ajax_enabled
+   *   Whether this view uses AJAX.
+   *
+   * @return bool
    *   TRUE if AJAX should be used, FALSE otherwise.
    */
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Version: 8.1.x-dev » 8.0.x-dev
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Thanks for trying, but this patch is not correct:

  1. +++ b/core/modules/views/src/ViewExecutable.php
@@ -604,10 +604,13 @@ public function usePager() {
+   * If AJAX is used, paging, table sorting and exposed filters will be fetched

    If you are going to fix this line (which is good), please also add a comma after "sorting". 

  2. +++ b/core/modules/views/src/ViewExecutable.php
@@ -604,10 +604,13 @@ public function usePager() {
+   * @param bool $ajax_enabled
+   *   Whether this view uses AJAX.
+   *
+   * @return bool
    *   TRUE if AJAX should be used, FALSE otherwise.
    */
   public function setAjaxEnabled($ajax_enabled) {

    Please be more careful when making patches like this to read the code. There is no return value for this function.

    Changing the @param line to the correct variable name is good. But the docs line that was there is (in my opinion) better than the new line you added, and there is no return value, so adding @return is incorrect.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review
StatusFileSize
new 2603348-2.patch727 bytes

@jhodgdon

Sorry, that was careless mistake from my side. I have changed that. Here is the following changes,

-   * If AJAX is used, paging, tablesorting and exposed filters will be fetched
+   * If AJAX is used, paging, table sorting, and exposed filters will be fetched
    * via an AJAX call rather than a page refresh.
    *
-   * @param bool $use_ajax
+   * @param bool $ajax_enabled
    *   TRUE if AJAX should be used, FALSE otherwise.

Status: Needs review » Needs work

The last submitted patch, 6: 2603348-2.patch, failed testing.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review

The last submitted patch, 2: 2603348-1.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks, looks good!

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Issue tags: +rc eligible

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2603348-2.patch, failed testing.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Reviewed & tested by the community

  • catch committed 7b3d444 on 8.1.x 
    Issue #2603348 by rakesh.gectcr, jhodgdon: Comment block contains wrong...

  • catch committed 2ed8b90 on 
    Issue #2603348 by rakesh.gectcr, jhodgdon: Comment block contains wrong...
catch’s picture

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

Committed/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!

Status: Fixed » Closed (fixed)

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