-
-
Notifications
You must be signed in to change notification settings - Fork 3
Add singleton-based plugin loading mechanism #16
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
Conversation
Introduce Nodeinfo class with centralized initialization pattern: - Singleton pattern with get_instance() for global access - Consolidated hook registration in register_hooks() - Admin hooks separated into register_admin_hooks() - Static activate/deactivate methods for plugin lifecycle - Simplified main plugin file to use new class
Add backwards compatibility handler using apply_filters_deprecated() to map the old wellknown_nodeinfo_data filter to nodeinfo_discovery.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the plugin initialization to use a centralized singleton pattern, consolidating previously scattered initialization logic into a single Nodeinfo class. The changes simplify the main plugin file and provide a more maintainable architecture with clear separation of concerns.
- Introduced
Nodeinfosingleton class to manage plugin lifecycle and hook registration - Consolidated hook registration from multiple functions into organized class methods
- Migrated activation/deactivation logic to static class methods
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| nodeinfo.php | Simplified main plugin file to use new singleton pattern via nodeinfo_plugin() function and class-based activation/deactivation hooks |
| includes/class-nodeinfo.php | New singleton class that centralizes plugin initialization, hook registration, and lifecycle management with separated admin and frontend hooks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move load_plugin_textdomain to init hook for proper timing - Add priority 9 to integration hooks to maintain original behavior
WordPress handles translations automatically for plugins on WordPress.org. Remove pot file, Domain Path header, and load_textdomain method.
Integrations only register filters, so they don't need to wait for the init hook. This removes the need for priority handling.
Tests cover: - Singleton pattern behavior - REST routes registration - Integration registration - Rewrite rules - Deprecated filter handling - WebFinger and admin hooks registration
- Use correct route pattern for nodeinfo2 endpoint - Enable permalinks before testing rewrite rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Hook init to plugins_loaded action for proper timing - Add is_admin() check before registering admin hooks - Call init() in activate() to ensure integrations are loaded - Update activate() docblock to better describe its purpose - Add proper test cleanup for global variables - Clarify changelog wording about deprecated filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Document why activate() calls both init() and add_rewrite_rules() - Add test for init() guard clause preventing double initialization - Add test for activate() method - Add test for deactivate() method
Instead of calling add_rewrite_rule() on every init hook, use the rewrite_rules_array filter which only fires during flush operations. This is more efficient as rules are cached in the database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Introduce Nodeinfo class with centralized initialization pattern: