Attached patch fixes coding standard issues.

Problem/Motivation

(EDIT)
The patch is quite old and currently is outdated.
The full list of coding standard issues could be found by looking at the output of the automated tests at https://www.drupal.org/node/3287/qa

Proposed resolution

Patches should NOT try to fix all issues at one time - instead, they should focus on just one type of coding standards problem and fix that one type of problem for Redirect. Note that there are some issues that should probably NOT be fixed. Let's focus on getting the important things done first, like missing @param and documentation comments and solve them as child issues.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 phpcs-redirect-august-8-2023-issue-2850121.txt49.75 KBKristen Pol
#7 2850121-coding_standards-7.patch8.92 KBandralex
#6 2850121-coding_standards-6.patch9.46 KBandralex
#5 2850121-coding_standards-5.patch19.33 KBandralex
fix_coding_standard_redirect_0.patch3.96 KBSubhransu.addweb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

denisha.addweb created an issue. See original summary.

andralex’s picture

andralex CreditAttribution: andralex at EPAM Systems for EPAM Systems commented
Issue summary: View changes
andralex’s picture

andralex CreditAttribution: andralex at EPAM Systems for EPAM Systems commented
Status: Needs review » Needs work
Issue tags: +Coding standards
solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

1. redirects.view -- should this file be a .php file or something? I don't know what a .view file is.

2. redirect.admin.inc calls redirect_build_filter_query($query, array('w.message'), $keys);. I get the following error from Intelliphense:

Expected type 'QueryAlterableInterface'. Found 'QueryExtendableInterface'.

So maybe redirect_build_filter_query should have a different parameter type hint?

3. redirect.install

// Limit the number of characters used by the index.
// That allows avoiding the risk to have a PDOException
// caused the DB index limitations of some databases.

Should this be caused by the DB index limitations of some databases?

andralex’s picture

andralex CreditAttribution: andralex at EPAM Systems for EPAM Systems commented
FileSize
2850121-coding_standards-5.patch19.33 KB

Here is a patch that fixes issues from #4

andralex’s picture

andralex CreditAttribution: andralex at EPAM Systems for EPAM Systems commented
Status: Needs work » Needs review
FileSize
2850121-coding_standards-6.patch9.46 KB

Here is an updated patch with correct encoding and line endings.

andralex’s picture

andralex CreditAttribution: andralex at EPAM Systems for EPAM Systems commented
FileSize
2850121-coding_standards-7.patch8.92 KB

Removed changes from .install file as it is part of 7.x-1.x branch.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented

@andralex Just FYI, 1.x and 2.x branches are the same. There is was literally no difference between the two (until recently, when a couple commits were only applied to 1.x), and that 1.x is actually used. We should target 1.x until there's actually a reason to change to 2.x.

andralex’s picture

andralex CreditAttribution: andralex at EPAM Systems for EPAM Systems commented
Version: 7.x-2.x-dev » 7.x-1.x-dev

Hello @solideogloria,

I've changed the target version to 7.1.

solideogloria’s picture

solideogloria CreditAttribution: solideogloria commented
Status: Needs review » Reviewed & tested by the community
Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital commented
Assigned: Unassigned » Kristen Pol

Assigning to myself as I'm triaging all RTBC issues.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital commented
Assigned: Kristen Pol » Unassigned
Status: Reviewed & tested by the community » Needs work
Related issues: +#3136894: Missing doc comment, +#3136904: Line exceeds 80 characters
FileSize
phpcs-redirect-august-8-2023-issue-2850121.txt49.75 KB

Thanks for the issue, patches, and reviews.

Unfortunately, this is definitely very outdated. See attached output from:

phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml .

Also note 2 related issues:

#3136894: Missing doc comment
#3136904: Line exceeds 80 characters

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Salsa Digital commented
Status: Needs work » Postponed
Related issues: +#3136895: Missing parameter type

Postponing this one for now until some others get handled first:

#3136894: Missing doc comment
#3136904: Line exceeds 80 characters
#3136895: Missing parameter type