Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Info about Lazy Subscriber #273

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Info about Lazy Subscriber #273

wants to merge 3 commits into from

Conversation

Fedik
Copy link
Member

@Fedik Fedik commented Jun 15, 2024

User description

information about Lazy Subscriber, with example.

PR:


PR Type

documentation


Description

  • Added documentation for the LazyServiceSubscriber decorator in the advanced plugin features section.
  • Provided an example of how to use the lazy subscriber decorator for plugins with heavy dependencies.
  • Explained the purpose and usage of Joomla\CMS\Event\LazySubscriberInterface.
  • Updated migration notes to include information about the new lazy decorator for plugins.

Changes walkthrough 📝

Relevant files
Documentation
advanced-plugin-features.md
Add documentation for Lazy Subscriber decorator in plugins

docs/building-extensions/plugins/advanced-plugin-features.md

  • Added detailed documentation about the LazyServiceSubscriber
    decorator.
  • Provided an example usage of the lazy subscriber decorator for plugins
    with heavy dependencies.
  • Explained the purpose and usage of
    Joomla\CMS\Event\LazySubscriberInterface.
  • +66/-0   
    new-features.md
    Document new lazy decorator feature in migration notes     

    migrations/51-52/new-features.md

  • Added a new section about the Lazy Subscriber decorator for plugins.
  • Provided a link to the detailed documentation and the relevant PR.
  • +9/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added documentation Improvements or additions to documentation Review effort [1-5]: 2 labels Jun 15, 2024
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5]

    2

    🧪 Relevant tests

    No

    🔒 Security concerns

    No

    ⚡ Key issues to review

    Clarity in Documentation:
    Ensure that the documentation clearly explains the conditions under which using a LazyServiceSubscriber is beneficial, especially the scenarios where it does not make sense to use it.

    Example Consistency:
    Verify that the example provided in the documentation accurately reflects the typical use cases and that the syntax and API usage are correct.

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Clarity
    Clarify the requirement for implementing the LazySubscriberInterface to use LazyServiceEventListener

    Clarify the sentence to specify that the plugin should implement the
    LazySubscriberInterface to use the LazyServiceEventListener.

    docs/building-extensions/plugins/advanced-plugin-features.md [11]

    -Additionally, the interface `Joomla\CMS\Event\LazySubscriberInterface` allows to use [`LazyServiceEventListener`](https://github.com/joomla-framework/event/blob/2.0-dev/src/LazyServiceEventListener.php), however the plugin should implement it on its own.
    +Additionally, the plugin should implement the interface `Joomla\CMS\Event\LazySubscriberInterface` to use [`LazyServiceEventListener`](https://github.com/joomla-framework/event/blob/2.0-dev/src/LazyServiceEventListener.php).
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies a need for clearer documentation regarding the implementation requirements for LazySubscriberInterface, which is crucial for correct usage.

    8
    Readability
    Improve readability by breaking down a complex sentence

    Improve readability by breaking down the sentence into two separate sentences.

    docs/building-extensions/plugins/advanced-plugin-features.md [9]

    -Note, that use of lazy decorator does not make sense when the plugin subscribed to the events that happening regularly, like `onAfterInitialise`.
    +Note that the use of the lazy decorator does not make sense when the plugin is subscribed to events that happen regularly, like `onAfterInitialise`.
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion improves readability by simplifying a complex sentence, making it easier to understand, which is beneficial but not critical.

    6
    Formatting
    Remove extra blank lines to maintain consistent formatting

    Remove the extra blank lines to maintain consistency in formatting.

    migrations/51-52/new-features.md [18-21]

     PR: [43575](https://github.com/joomla/joomla-cms/pull/43575)
    -
     #### New lazy decorator for Plugins
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion correctly addresses a minor formatting issue to improve the consistency of the document, which enhances readability but is not a major concern.

    5

    @Fedik Fedik marked this pull request as draft June 15, 2024 12:55
    @robbiejackson robbiejackson requested a review from pe7er June 20, 2024 10:50
    @robbiejackson
    Copy link
    Contributor

    Woops, sorry @Fedik @pe7er - I clicked on the review button accidentally.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation Review effort [1-5]: 2
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants