Skip to content

[Feature]: Preparation for WordPress org release #57

Open
theMasudRana wants to merge 40 commits intodevelopfrom
feat/prepair-for-wp-org
Open

[Feature]: Preparation for WordPress org release #57
theMasudRana wants to merge 40 commits intodevelopfrom
feat/prepair-for-wp-org

Conversation

@theMasudRana
Copy link
Collaborator

@theMasudRana theMasudRana commented Feb 24, 2026

Description: Prepare the plugin for WordPress.org release

  • Create readme.txt file
  • Add .pot file
  • Add .distignore file
  • Public Repo Link added.
  • External Placeholder Images removed
  • Added Direct Access Prevention

Issue: https://github.com/rtCamp/support/issues/462#issuecomment-3982065178

@theMasudRana theMasudRana self-assigned this Feb 24, 2026
@theMasudRana theMasudRana requested review from danish17 and removed request for danish17 February 26, 2026 13:53
@theMasudRana theMasudRana marked this pull request as ready for review March 2, 2026 14:32
@up1512001 up1512001 requested a review from Copilot March 3, 2026 09:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prepares the Carousel Kit plugin for a WordPress.org release by adding WP.org-required metadata/artifacts, localizing assets, and hardening direct access.

Changes:

  • Adds WordPress.org packaging assets (readme.txt, .distignore) and localization template (POT) plus an npm script to generate it.
  • Replaces externally-hosted placeholder images in example patterns with bundled local assets.
  • Adds direct-access prevention and an index.php “silence is golden” file.

Reviewed changes

Copilot reviewed 10 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
readme.txt Adds WordPress.org readme metadata, description, screenshots, changelog.
package.json Adds repository/bugs metadata and an i18n POT generation script.
languages/carousel-kit.pot Adds translation template for strings.
index.php Adds “silence is golden” index to prevent directory listing.
examples/patterns/logo-showcase.php Switches pattern images from external placeholders to bundled SVGs.
examples/patterns/hero-carousel.php Switches hero slides from external placeholders to bundled WEBPs.
examples/data/images/logo-placeholder-*.svg Adds bundled placeholder logo SVG assets.
composer.json Removes a production dependency from require.
composer.lock Updates lockfile to reflect removed production dependency.
carousel-kit.php Updates license header and adds direct access guard.
Makefile Adjusts dist exclusions (README.md no longer excluded).
.distignore Adds ignore rules for WordPress.org distribution packaging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 34 to 36
"require": {
"php": "^8.2",
"scrivo/highlight.php": "^9.18"
"php": "^8.2"
},
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

composer.json requires PHP ^8.2, but the plugin header (carousel-kit.php) and readme.txt both declare Requires PHP: 7.4. For WordPress.org, these should be aligned with the actual runtime requirement; either lower composer.json's PHP constraint/platform or bump the plugin/readme requirement to 8.2 if that's the real minimum.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theMasudRana

Have we tested this plugin with PHP 7.4?
If not, then whichever is the lowest version of PHP you've tested this plugin with, that should be the required PHP version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gagan0123, Sorry about that — we haven’t tested this plugin on PHP 7.4, and declaring 7.4 support was my mistake.
We’re keeping the minimum required PHP version at 8.2 (the lowest version currently validated), and our CI is also configured for PHP 8.2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, checked that the readme.txt was updated for the same, but README.md is still saying 7.4 as min requirement
Ref: https://github.com/rtCamp/carousel-kit/blob/feat/prepair-for-wp-org/README.md#requirements

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that, @gagan0123,
I've updated the README.md as well to reflect PHP 8.2 as the minimum required version.

Copy link
Member

@up1512001 up1512001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theMasudRana Added few suggestions.

carousel-kit.php Outdated
@@ -8,14 +8,18 @@
* Author: rtCamp
* Author URI: https://rtcamp.com
* Contributors: iamdanih17, immasud
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add rtcamp as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added rtcamp as a contributor.

carousel-kit.php Outdated
exit; // Exit if accessed directly.
}

define( 'CAROUSEL_KIT_PATH', untrailingslashit( plugin_dir_path( __FILE__ ) ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if possible wrap this all const into function and add namespace to this file.
reference: https://github.com/rtCamp/OneUpdate/blob/e5920c452fffc70a2d7d922a2c80d44ff065a216/oneupdate.php#L29

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion — updated this file to use a namespace and moved all plugin constant definitions into a dedicated constants() function (called during bootstrap). Behavior remains the same, but initialization is now cleaner and more consistent with our standards.

carousel-kit.php Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add file exist check if someone downloads from GH then activating this plugin should give admin notice instead of fatal error.

https://github.com/rtCamp/OneAccess/blob/b8ac1e297c90949e98c2a1ad696380c8b033a15c/inc/Autoloader.php#L61

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored bootstrap to a dedicated inc/Autoloader.php class and switched loading flow to Autoloader::autoload(). It now handles missing vendor/autoload.php gracefully with admin/network notices and avoids fatal activation errors.

image

package.json Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to remove version from composer & package file to avoid maintaining it on multiple places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the version from package.json and package-lock.json
Plugin version is not added in the composer.json file.

3. Query Loop integration for dynamic content
4. Frontend carousel with autoplay

== Changelog ==
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in plugin root we should have CHANGELOG.md https://github.com/rtCamp/godam/blob/main/CHANGELOG.md

and add just reference of that be adding at latest 2 release info

https://github.com/rtCamp/godam/blob/2456c02e667587571eb8be45f3df8f9d55e449f3/readme.txt#L247C1-L300C83

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the CHANGELOG.md file in the plugin root.
Added the full changelog reference at the end of the current changelog.

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@up1512001

The CHANGELOG.md file has been in the branch for more than a week, why you were not able to find it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gagan0123 I was just saying for public plugins in readme.txt we should only include 2-3 latest changelog and at end mention our CHANGELOG.md file so that readme.txt don't get too big and it will save copy-paste work.

readme.txt Outdated
* Query Loop integration
* RTL and accessibility support

== Upgrade Notice ==
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the upgrade notice section because we have not published any breaking changes, as per recommendation we don't need this.

@theMasudRana theMasudRana requested a review from up1512001 March 3, 2026 21:47
Copy link
Member

@up1512001 up1512001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor Nits:

composer.json Outdated
"test": "phpunit",
"test:unit": "phpunit --testsuite unit",
"test:coverage": "phpunit --coverage-html tests/_output/coverage"
"format": "vendor/bin/phpcbf",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why vendor prefix added??

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vendor/bin/ prefix was added to ensure the scripts could resolve the correct binary paths explicitly. However, this was unnecessary — Composer automatically prepends vendor/bin to the PATH when executing scripts, so bare binary names work just fine. It has been removed to keep the scripts cleaner.

@@ -21,15 +21,14 @@ parameters:
- tests/phpstan/constants.php
paths:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add uninstall.php

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theMasudRana theMasudRana requested a review from up1512001 March 5, 2026 20:38
Copy link
Member

@up1512001 up1512001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theMasudRana few nits, then good to go from my end 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link seems to be incorrect https://github.com/rtCamp/carousel-system-interactivity-api/releases

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the link.

package.json Outdated
"@types/wordpress__block-editor": "15.0.0",
"@types/wordpress__blocks": "12.5.18",
"@wordpress/scripts": "^31.2.0",
"@wordpress/scripts": "31.5.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to version lock & not allowing minor/patch version updates??

was anything breaking?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! No, nothing was breaking - it was just inconsistent.
Updated packages to use ^ for semver ranges now.

- vendor/
analyseAndScan:
- node_modules (?)
- node_modules/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node_modules might not always exists so would prefer ? instead of /

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added (?) for node_modules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants