Problem/Motivation

See #3504381: [meta] Convert Template Preprocess hooks to OOP equivalent

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3519257-nr-bot.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3519257

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:

Comments

berdir created an issue. See original summary.

nicxvan’s picture

Quick comment on the constructor.

Any reason behind the file and function names being inverted?

berdir’s picture

> Any reason behind the file and function names being inverted?

I assume you mean classname?

It's the same with all the Preprocess classes that I added. Class is FooPreprocess, and then the methods are preprocessFoo. with ThemeProcess it was just so that Foo wasn't the same thing so far.

Makes sense to me, for the class name it's just following naming rules (repeat last part of namespace if otherwise not providing enough context/being unique enough), while methods are verbThing.

nicxvan’s picture

Status: Needs review » Needs work

Yes, this needs a rebase, the other issue has the same failure.

berdir’s picture

Status: Needs work » Needs review

Rebased.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Taking a guess here.

But applied the MR, setup a paginated view,
Pagination appears to be working.

The one thread about brackets probably can be a coding standard issue as sure there are other instances that may need to be fixed.

nicxvan’s picture

Yeah I also carefully reviewed this, both with and without the zebra command git diff 11.x --color-moved=dimmed_zebra --color-moved-ws=ignore-all-space

The changes are comments and injections.

I retract my comments on the MR as @smustgrave and @berdir pointed out they belong in a coding standards issue.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

berdir’s picture

Status: Needs work » Needs review

Rebased, it's been a while and had a bunch of conflicts with the field preprocess issue, so leaving at needs review for a confirmation.

nicxvan’s picture

One suggestion on the deprecation and one question on the callback.

nicxvan’s picture

Status: Needs review » Reviewed & tested by the community

Went through this carefully, looks the same as the previous rtbc besides updating the deprecation.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

  • catch committed d417b2ab on 11.x
    Issue #3519257 by berdir, nicxvan, smustgrave: Convert...

catch’s picture

Status: Fixed » Closed (fixed)

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