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

Add docu for new update event #340

Merged
merged 16 commits into from
Jan 27, 2025
Merged

Add docu for new update event #340

merged 16 commits into from
Jan 27, 2025

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Nov 25, 2024

User description

Docs for joomla/joomla-cms#44516


PR Type

documentation


Description

  • Added documentation for new installer plugin events: onInstallerBeforePackageDownload and onInstallerBeforeUpdateSiteDownload.
  • Provided detailed descriptions and examples for using these events in the Joomla CMS.
  • Updated migration notes to include information about the new event for fetching update site URLs.

Changes walkthrough 📝

Relevant files
Documentation
installer.md
Document new installer plugin events and examples               

docs/building-extensions/plugins/plugin-events/installer.md

  • Added documentation for installer events.
  • Described onInstallerBeforePackageDownload event.
  • Described onInstallerBeforeUpdateSiteDownload event.
  • Provided examples for both events.
  • +63/-0   
    new-features.md
    Add documentation for new before download update site event

    migrations/52-53/new-features.md

  • Documented new event for update site URL fetching.
  • Provided example usage of the new event.
  • +13/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    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 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Documentation Issue
    The code block for onInstallerBeforePackageDownload example is not properly closed with triple backticks, which may affect documentation rendering

    Copy link
    Contributor

    qodo-merge-pro bot commented Nov 25, 2024

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

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    ✅ Fix markdown syntax error that breaks document formatting by adding missing code block closure
    Suggestion Impact:The missing closing code block marker was added, correcting the markdown syntax error.

    code diff:

    @@ -34,6 +35,7 @@
     {
         $event->updateUrl($event->getUrl() . '?auth=foo');
     }
    +```

    Add missing closing code block marker () after the first example, which is causing
    the second section to be incorrectly formatted as part of the code block.

    docs/building-extensions/plugins/plugin-events/installer.md [33-38]

     ```php
     public function onInstallerBeforePackageDownload(\Joomla\CMS\Event\Installer\BeforePackageDownloadEvent $event): void
     {
         $event->updateUrl($event->getUrl() . '?auth=foo');
     }
    +```
     
     ## onInstallerBeforeUpdateSiteDownload
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The missing code block closure is a critical markdown syntax error that breaks the entire document's formatting, making the documentation hard to read and potentially confusing for users. Adding the missing "```" marker is essential for proper rendering.

    9

    💡 Need additional feedback ? start a PR chat

    @robbiejackson
    Copy link
    Contributor

    Could you also add a line into the List of Plugin Events index page? It's in building-extensions/plugins/plugin-events/index.md

    Thanks

    @laoneo
    Copy link
    Member Author

    laoneo commented Nov 27, 2024

    Like this d57b584?

    @robbiejackson
    Copy link
    Contributor

    Yes, that's right for the index page.

    On the Installer Events page you need to close the first section of php code as the second event is included within the code block - see http://pr-340.manual.joomlacode.org/docs/next/building-extensions/plugins/plugin-events/installer/#onInstallerBeforeUpdateSiteDownload

    Also in the text "before a package of an extension is downloaded" - I wasn't sure if this referred specifically to a Joomla package (ie group of extensions packaged together into a Joomla package) or just to a zipped-up file of a Joomla extension (not necessarily a Joomla package). Some text to make that clear would be very helpful.

    @laoneo
    Copy link
    Member Author

    laoneo commented Nov 27, 2024

    @robbiejackson please check 90cf229

    @robbiejackson
    Copy link
    Contributor

    I wondered why the links from the List of Plugin Events index page weren't working properly, then found from https://docusaurus.io/docs/next/markdown-features/toc#heading-ids that docusaurus generates the anchors in lower case if you don't specify explicitly the heading id.

    With those fixed, I think this PR is good to go once the code is merged into Joomla 5.3. The relevant bits need to be duplicated to the 4.4 and 5.2 versioned docs, which you can either do yourself, or I'm happy to do on your behalf - just let me know.

    @laoneo
    Copy link
    Member Author

    laoneo commented Nov 27, 2024

    Thank you for your help on this. And sure go for your suggested changes in 4.4 and 5.2.

    @HLeithner HLeithner merged commit 56600d5 into main Jan 27, 2025
    3 checks passed
    @HLeithner
    Copy link
    Member

    thx

    @HLeithner HLeithner deleted the update branch January 27, 2025 06:50
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants