Problem/Motivation

There are PHPCS remarks, which can't be fixed automatically and requires manual fixing. The automatic fixes would be handled at issue #3565134, but in this task I propose to resole manual ones, so the MR would be less overwhelming and easier to review.

Steps to reproduce

The example error pipeline: https://git.drupalcode.org/issue/field_tools-3469906/-/jobs/2533888

Proposed resolution

Resolve the PHPCS errors in accordance with @joachim's remark (https://www.drupal.org/project/field_tools/issues/3469906#comment-16394195), or provide correct ignore comments for PHPCS to avoid false-positives.

Remaining tasks

Make the pipeline of PHPCS green (run existing codebase without remarks).

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork field_tools-3565138

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

konot created an issue. See original summary.

konot opened merge request !24

konot’s picture

konot commented
Assigned: konot » Unassigned
Status: Active » Needs review

I have resolved the PHPCS errors that were not intended to be resolved automatically by phpcbf.

However, there are some errors which I have not fixed as they were made intentionally and should be reviewed before changing, if they were left intentionally:
1. Errors in `field_tools.admin.inc` - the file is deprecated, and the errors would go away after the file is removed.
2. Errors in `field_tools.module` - relating code after `return` and functions naming: it seems that someone left them intentionally, and I believe that they should be checked before removing, as I don't know the reason people left this code (maybe it usefull for patches or anything like that).

I am looking for the MR review. Let me know if there is anything to fix, and I'll take care of it. Thanks!

joachim’s picture

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

LGTM.

Thanks for your attention to detail!

andrii momotov made their first commit to this issue’s fork.

andrii momotov’s picture

andrii momotov commented
Status: Reviewed & tested by the community » Fixed

MR was merged. Thanks!

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

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

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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