Problem/Motivation

Resolve phpcs issues in the 2.0.x and 2.1.x branches.

These are important to resolve because not marking nullable with ? is deprecated in PHP 8.4 and produces a warning.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork metatag-3484975

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

damienmckenna created an issue. See original summary.

ankitv18 made their first commit to this issue’s fork.

ankitv18 opened merge request !152

kul.pratap’s picture

kul.pratap commented
Assigned: Unassigned » kul.pratap
kul.pratap’s picture

kul.pratap commented
Assigned: kul.pratap » Unassigned
Status: Active » Needs review

I have solved all the phpcs error, will i also solve phpstan and phpunit error to pass pipeline fully green?

a.aaronjake’s picture

a.aaronjake commented
Status: Needs review » Reviewed & tested by the community

Hi @kul.pratap.,

I have re-ran the pipeline, confirmed the phpcs errors are now fixed and the pipeline passed: Gitlab CI pipeline passed phpcs

Will now move this to RTBC since this issue is for the phpcs only.

Thanks,
Jake

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Needs work

Please don't mark something RTBC if you haven't checked the test pipeline, because unfortunately these changes broke a lot of tests. Also, it seems to have added a cspell issue.

ankitv18 opened merge request !154

ankitv18 changed the visibility of the branch 3484975-resolve-phpcs-issues to hidden.

ankitv18 opened merge request !155

ankitv18 changed the visibility of the branch 3484975-phpcs-fixes-2.1.x to hidden.

ankitv18 opened merge request !157

kul.pratap’s picture

kul.pratap commented
Status: Needs work » Needs review

All PHPCS issues have been resolved in versions 2.0.x and 2.1.x. Please review.

ptmkenny’s picture

ptmkenny commented
Issue summary: View changes
Priority: Normal » Major
Status: Needs review » Needs work

MR157 needs the merge conflicts fixed.

Also added a note to the issue summary that this is an important issue because this fixes deprecation warnings in PHP 8.4.

kul.pratap’s picture

kul.pratap commented
Status: Needs work » Needs review
ptmkenny’s picture

ptmkenny commented
Status: Needs review » Needs work

@kul.pratap Please make sure the test icon is green before setting to "Needs review." On the 2.0.x branch (3484975-phpcs-fixes), phpunit is failing, but it is not failing in dev (you can check the 2.0.x GitLab CI pipeline from the module page, which is currently here: https://git.drupalcode.org/project/metatag/-/pipelines/332334)

Since phpunit passes in dev but fails in 3484975-phpcs-fixes, then the 3484975-phpcs-fixes is broken and needs to be fixed before it can be reviewed.

kul.pratap’s picture

kul.pratap commented

@ptmkenny As the issue summary only mentions PHPCS, I have resolved the PHPCS errors and changed the issue state to 'Needs Review.' However, since you mentioned that I need to resolve PHPUnit errors and pass the pipeline, I am now working on it.

kul.pratap’s picture

kul.pratap commented
Assigned: Unassigned » kul.pratap

kul.pratap changed the visibility of the branch 3484975-phpcs-fixes to hidden.

kul.pratap changed the visibility of the branch 3484975-phpcs-fixes-2.0.x to hidden.

kul.pratap changed the visibility of the branch 3484975- to hidden.

kul.pratap changed the visibility of the branch 3484975-phpcs-fixes-2.0.x to active.

kul.pratap’s picture

kul.pratap commented
Assigned: kul.pratap » Unassigned
Status: Needs work » Needs review

@ptmkenny you can check MR!154 now it is passing.

ptmkenny’s picture

ptmkenny commented
Status: Needs review » Needs work

As the issue summary only mentions PHPCS, I have resolved the PHPCS errors and changed the issue state to 'Needs Review.' However, since you mentioned that I need to resolve PHPUnit errors and pass the pipeline, I am now working on it.

Yes, this issue is to fix the PHPCS test. However, the phpunit tests are passing on dev, so if as a result of fixing the PHPCS test, the phpunit test breaks, it needs to be fixed here.

Thank you for fixing MR!154. However, as stated in the issue summary, we need to fix PHPCS issues in both the 2.0.x and the 2.1.x branches. It seems you hid the branch for 2.1.x--(3484975-phpcs-fixes-2.1.x)-- but this branch also needs to have an MR with the same fixes as 2.0.x. The reason is that both 2.0.x and 2.1.x are currently supported by the module. (Also, work generally must always be committed to the latest dev branch, which in this case is 2.1.x-dev, so there is no way we can commit 2.0.x without 2.1.x.)

Setting back to "Needs work" because we need an MR for 2.1.x-dev as well with green tests.

kul.pratap’s picture

kul.pratap commented

Ok i will work on it

kul.pratap opened merge request !166

kul.pratap opened merge request !167

kul.pratap closed merge request !166

damienmckenna closed merge request !154

damienmckenna closed merge request !167

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Resolve phpcs issues in 2.0.x / 2.1.x » Resolve phpcs issues
Assigned: Unassigned » damienmckenna

damienmckenna opened merge request !210

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3548158: Plan for Metatag 2.2.1

  • damienmckenna committed fd61ff8a on 2.2.x 
    Issue #3484975 by ankitv18, damienmckenna, ptmkenny, kul.pratap, a....
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: damienmckenna » Unassigned
Status: Needs work » Fixed

Committed. Thank you all.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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