Problem/Motivation

 * The remainder of this documentation will assume Annotation-based discovery,
 * since this is the most common method.
 *
 * @subsection sub_manager Defining a plugin manager class and service
 * To define an annotation-based plugin manager:
 * - Choose a namespace subdirectory for your plugin. For example, search page
 *   plugins go in directory Plugin/Search under the module namespace.
 * - Define an annotation class for your plugin type. This class should extend
 *   \Drupal\Component\Annotation\Plugin, and for most plugin types, it should
 *   contain member variables corresponding to the annotations plugins will
 *   need to provide. All plugins have at least $id: a unique string
 *   identifier.

Everything that talks about annotations should be attributes instead, in the above and the rest of that section.

Tagging as novice but this line will need more substantial changes:

it should
 *   contain member variables corresponding to the annotations plugins will
 *   need to provide.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#18 3613569-nr-bot_rok57l2p.txt549 bytesneeds-review-queue-bot
#16 3613569-nr-bot_5hc4mx5c.txt549 bytesneeds-review-queue-bot
#13 3613569-nr-bot_2ro248ye.txt549 bytesneeds-review-queue-bot
#11 3613569-nr-bot_3g69cp3g.txt549 bytesneeds-review-queue-bot
#6 3613569-nr-bot_onuxwk80.txt549 bytesneeds-review-queue-bot

Issue fork drupal-3613569

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

joachim created an issue. See original summary.

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

charlliequadros opened merge request !16546

charlliequadros’s picture

charlliequadros commented

Hi @joachim

I’ve made the requested changes. Please let me know if anything else is needed.

charlliequadros’s picture

charlliequadros commented
Status: Active » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3613569-nr-bot_onuxwk80.txt549 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

joachim’s picture

joachim commented

Looks great!

I'm not sure why the bot isn't happy -- could it be line length?

Just one thing I think needs changing.

charlliequadros’s picture

charlliequadros commented
Assigned: Unassigned » charlliequadros
charlliequadros’s picture

charlliequadros commented

Hi @joachim

I’ve made the change you suggested, and it’s now ready for review.
Regarding the bot, I believe it was a false positive.

charlliequadros’s picture

charlliequadros commented
Assigned: charlliequadros » Unassigned
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3613569-nr-bot_3g69cp3g.txt549 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

charlliequadros’s picture

charlliequadros commented
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3613569-nr-bot_2ro248ye.txt549 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

charlliequadros’s picture

charlliequadros commented
Status: Needs work » Needs review
charlliequadros’s picture

charlliequadros commented

The build is failing because only one file was changed, and that file ends with `api.php`. PHPStan ignores these files, so it ends up with no files to analyse and returns an error.

This is the expected behaviour in this scenario. If this is not the desired behaviour, we can create an issue to investigate and adjust it.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3613569-nr-bot_5hc4mx5c.txt549 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

charlliequadros’s picture

charlliequadros commented
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3613569-nr-bot_rok57l2p.txt549 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

quietone’s picture

quietone commented
Issue tags: +no-needs-review-bot

The bot is complaining because there is a bug in the Drupal core commit checks, commit-code-check.sh, when there are only *.api.php files in the MR the PHPStan checks fail.

charlliequadros’s picture

charlliequadros commented
Status: Needs work » Needs review
danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented

What do we think about expanding the scope of this one slightly to just remove all references to annotations in that file (unless they're needed for reason). I've just been reviewing it and the changes as they are all look good and are probably ready for RTBC, but there is still 38 references to annotations in that file. My vote would be to do it in one hit, but if people disagree maybe we make a follow up and RTBC this one?

charlliequadros’s picture

charlliequadros commented

Hi @danielveza, @joachim

Personally, I would prefer opening a separate issue, mainly to keep the changes separated and easier to track.
However, I'm not sure if there is any real advantage to doing that or if it makes much difference to the Drupal workflow.
I'm happy with either approach.

smustgrave’s picture

smustgrave commented

+1 to knocking it all in one go. Current MR is pretty small so expanding wouldn't be a review load.