Problem/Motivation

See #3483037: [META] Add return types to hook implementations

Steps to reproduce

Find all occurrences in the baseline:

grep "Function .*_update_last_removed\\\\.* has no return type specified" core/.phpstan-baseline.php

Proposed resolution

Add : int to each of them.

This nifty command will find and replace all occurrences that come shortly after "Implements hook_update_last_removed()":
find . -type f -exec sed -i '/Implements hook_update_last_removed()/,/\(function .*_update_last_removed(.*)\) *{/!b;/\(function .*_update_last_removed(.*)\) *{/s/\(function .*_update_last_removed(.*)\) *{/\1: array {/' {} +

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3483050

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mstrelan created an issue. See original summary.

mstrelan opened merge request !9928

mstrelan’s picture

mstrelan commented
Status: Active » Needs review
bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs review » Reviewed & tested by the community

Another lveoly clearnup. Starting to repeat myself, but changes look good. When lookin in code, this is exactly what should have been changes. See no missed code. Count didnt add up initially, but that is because of the string in "update_test_last_removed.into.yml".

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs work

I too applied the diff and searched. I found all instances fixed but this needs to be rebased.

mstrelan’s picture

mstrelan commented
Status: Needs work » Postponed

Postponing on #3483040: Add array return type to all hook_removed_post_updates implementations as it doesn't make sense to rebase each of these, let's do one at a time.

mstrelan’s picture

mstrelan commented
Status: Postponed » Needs work

Needs a re-roll

Anonymous’s picture

Anonymous (not verified) commented

The issue seems to be worked on correctly and : int was added to all the required instances. The target branch is 56 commits behind the source branch and thus needs to be rebased.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

Hi @mstrelan
The pipeline you ran after rebasing is failing the PHPUnit (Functional) test.
Please take a look.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Thanks, but these were known random failures, unrelated to this issue. I've retried the pipeline and it passed.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

@mstrelan
Didn't know that those were known random failures. I thought some PHPUnit tests were failing causing the pipeline to fail and left a comment for you to look into it without checking further!

The pipeline is all green now!
The Changes look good, int return type was added to all hook_update_last_removed implementations and The baseline was updated.

Moving it to RTBC.

catch’s picture

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

Needs a rebase.

nicxvan’s picture

nicxvan commented

I did #3483065: Add void return type to all hook_form_alter, hook_form_FORM_ID_alter and hook_form_BASE_FORM_ID_alter implementations first , might as well wait until that gets in before doing this one.

nicxvan’s picture

nicxvan commented
Status: Needs work » Needs review

Credit

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

@nicxvan
The Rebase look fine, int return type was added to all hook_update_last_removed implementations and The baseline was updated and all tests are green.
Moving it to RTBC. Great work

  • catch committed 41281d7c on 11.1.x 
    Issue #3483050 by mstrelan, nicxvan: Add int return type to all...

  • catch committed 3d350b03 on 11.x 
    Issue #3483050 by mstrelan, nicxvan: Add int return type to all...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 11.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 11.1.x thanks!

catch closed merge request !9928

Anonymous’s picture

Anonymous (not verified) commented

Hi @catch,

I have been actively following and testing the issue to confirm its resolution. Once I verified that the issue was resolved, I also moved it to the "Reviewed and Tested by Community" state.

As I contributed to this process, I would appreciate it if my effort could be acknowledged with a credit. This recognition would be highly motivating and encourage me to continue contributing.

Thank you for your attention to this matter. I look forward to your response.

Best regards,
Akul

catch’s picture

catch
he/him
Primary language English
commented

@akulsaxena there's not enough in this issue for an RTBC to count towards issue credit. Sometimes an RTBC can, but it would need to be a more complex issue (and more complex RTBC).

See this page for some examples of what does and doesn't receive issue credit.

https://www.drupal.org/about/core/policies/maintainers/how-is-credit-gra...

Status: Fixed » Closed (fixed)

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