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

Several Enhancements & Bug Fixes #50

Merged
merged 7 commits into from
Mar 1, 2016
Merged

Several Enhancements & Bug Fixes #50

merged 7 commits into from
Mar 1, 2016

Conversation

jaswrks
Copy link
Contributor

@jaswrks jaswrks commented Mar 1, 2016

Summary of Changes

  • Modified the default build target (i.e., running just phing or phing build). This now excludes token replacements, POT generation, and it leaves an existing composer.lock intact. This is intended for contributors so that they can rebuild easily to test their changes. Referencing: Improve how Phing is used during testing of feature branches #49

  • Adding a new build target phing build-all, which will do what phing and/or phing build did in the previous version of our PSR build process; i.e., it will perform a full/complete build that runs all targets—intended for a lead developer.

  • Fixed a bug in POT generation. The translations directory should be created automatically if it does not yet exist.

  • Fixed a bug in the -config and -validate targets. Undefined properties within Phing are interpreted as string literals instead of as empty strings, which is what we had assumed in code. For instance, if ${project_other_phar_fileset_exclusions} (an optional property) is not defined, it was being read as the string literal ${project_other_phar_fileset_exclusions}, which can lead to unexpected consequences. This has been fixed by running a deeper config setup and validation, and then defaulting these properties to empty strings when they are not already defined.

  • Added a new internal build property: ${_project_text_domain}, which is derived from ${project_slug} for both the lite and pro versions. This property value will always strip away any -lite or -pro suffix, leaving us with a clean and consistent text domain for internal use.

  • Renamed the internal build property ${is_lite} to ${is_lite_build}.

  • Renamed the internal build property ${_is_lite_applicable} to ${_has_lite_build_props}.

  • Added a new reflexive sub-routine to the -pots target. Any instances of __(), _x(), _n(), or _nx() that do not already include a text-domain (either as a static value or with a variable), are forced to a static value matching the plugin's text-domain. This allows a developer to build a plugin without needing to type the text-domain over and over again. It will be added automatically each time you run phing build-all. I expect this to become a common practice in new projects based on the WP Sharks Core in the future, as I am not planning to use variables or constants for the text-domain moving forward; i.e., this will improve translation compatibility across-the-board by doing it this way.

    Note: If you have i18n calls that already specify a text-domain argument (either as a static value or as a variable, constant, etc.) those are left as-is. Also note that this does not interfere with the existing lite generation target where we absolutely force all static text-domain values whenever ${project_lite_text_domain_regex_replacement_pattern} is defined with a regex pattern.

  • New internal build property/conditional with name: ${_is_wp_sharks_core_theme_plugin}. This is only true if WP Sharks Core properties (seen below) exist in your .build.props file, and only when ${_is_wp_theme_plugin} is also true, which is an existing internal flag that we already had.

  • Adding new (optional) build properties:

    Defining any of these puts the build process into 'WP Sharks Core' theme/plugin mode.

    project_wp_sharks_core_required_version = 160229
    project_wp_sharks_core_tested_up_to_version = 160229
    project_wp_sharks_core_max_compatible_version  =
    
    • project_wp_sharks_core_required_version = Required version of the WP Sharks Core.

    • project_wp_sharks_core_tested_up_to_version = Last tested version of the WP Sharks Core.

    • project_wp_sharks_core_max_compatible_version = If this is defined, it can be used by the WP Sharks Core RV handler in order to prevent the plugin from loading if a newer core framework plugin is running on any given site.

      In this scenario, the site owner is shown a notice that informs them the plugin they are running is not yet compatible with the newer version of the WP Sharks Core they have, and suggests that they downgrade the WP Sharks Core or remove the old plugin.

      This is going to be rarely used (because we will try hard not to have BC breaks), but it gives us a graceful way to discontinue old plugins and prevent them from loading when/if the WP Sharks Core diverges in such a way that old plugins (those not worth updating) become unusable in newer versions of the WP Sharks Core.

      At the same time, it offers site owners a way to continue to use the older plugin if they want to, by providing them with instructions that explain how to downgrade the WP Sharks Core framework plugin to an older/compatible version if they want to run the older plugin.

  • Added support for docBlock-style WP plugin headers as seen in the second example here: https://codex.wordpress.org/File_Header ~ I expect this to become a common practice in WP Sharks Core plugin projects.

  • Improved logic and display in the -preamble build target.

  • Bug fix. Only apply static text-domain replacements during lite generation if ${project_lite_text_domain_regex_replacement_pattern} is defined and not empty.

@jaswrks
Copy link
Contributor Author

jaswrks commented Mar 1, 2016

@raamdev I ran some initial tests after these changes and things look good. However, I'm going to leave this labeled as in-progress for now until I have time to run some additional tests against existing repos that use the Phing build script; e.g., Comet Cache & CM would be good to test this branch with now.

If you run those tests before I do, feel free to merge this branch if you'd like to announce the new phing build behavior to everyone and let them start using it.

@raamdev
Copy link
Contributor

raamdev commented Mar 1, 2016

@jaswsinc writes...

Added a new internal build property: ${_project_text_domain}, which is derived from ${project_slug} for both the lite and pro versions. This property value will always strip away any -lite or -pro suffix, leaving us with a clean and consistent text domain for internal use.

I reviewed your changelog above--nice work! The only question I has was about stripping -lite or -pro from the text domain. Are we sure we want to do that? Is there any reason not to do that? I see that we're using comet-cache for both the Lite and Pro versions of the plugin, but what if (thinking about future-proofing here) we released a Pro version of a plugin on WordPress.org (something that, for example, interfaced with an external API that required a Pro key, and it was the Pro API key they needed to purchase, thereby allowing us to release the plugin itself on W.org)? If we did that, we'd need the Lite and Pro versions to have different TD Slugs (so that the plugins could be translated via translate.wordpress.org, which requires unique TD slugs).

I realize that might be an edge case, but it did come to mind as I read through the changes. I'm fine with whatever you think is best. :-)

@jaswrks
Copy link
Contributor Author

jaswrks commented Mar 1, 2016

I realize that might be an edge case, but it did come to mind as I read through the changes. I'm fine with whatever you think is best. :-)

Great points. I gave this some thought too, and I had some of the same feelings about this. After reading over your thoughts above, I gave it some more thought. Then I remembered that even if a scenario like that did arise, it won't change the fact that our goal is to have a single codebase for both a lite and pro version of a plugin. Therein lies the additional benefit of only ever needing a single text domain, because the lite version is always going to be a watered-down version of the pro codebase; i.e., the same translatable strings, minus anything that is pro-only.

If a translator wants to translate either version, they should be translating one POT file only, which will cover both the lite and pro versions. In other words, if you're going to spend the time building a translation, it shouldn't be a partial translation, it should be a full translation that works in either variation of the software. That way it works for everyone.

So for that reason (regardless of what they are doing at translate.wordpress.org), I think it's best to keep the text domain without any -lite|pro suffix. However, what we probably do need to change, is the way the POT is generated in our Phing build for the lite variation. At present, the lite POT is generating separately from the pro version, which cuts away any pro-only strings from the POT we ship in the lite version. I think that shouldn't be the case. The POT for both should be the same, just like the text domain.

@jaswrks
Copy link
Contributor Author

jaswrks commented Mar 1, 2016

The WP Sharks Core (and our Phing build) has been designed thus far to expect two things to change between the lite/pro variations of the software, and only these two things.

  • The plugin basename should be different. The pro version should have a -pro suffix; e.g., comet-cache-pro/comet-cache-pro.php or comet-cache-pro/plugin.php (preferred).
  • The namespace should be different. The pro version should live in a \Pro sub-namespace.

This allows both plugins to live inside the same WordPress installation, and then they can each detect each other and avoid the potential for conflicts if both happen to be active at the same time.

Everything else in the WP Sharks Core operates and enforces a standard of having a single text-domain, the same global variable reference, the same internal slug, and the same internal DB prefix and keys.

jaswrks pushed a commit that referenced this pull request Mar 1, 2016
Several Enhancements & Bug Fixes
@jaswrks jaswrks merged commit a9b95ff into 000000-dev Mar 1, 2016
@jaswrks jaswrks deleted the feature/wps branch March 1, 2016 22:03
@jaswrks
Copy link
Contributor Author

jaswrks commented Mar 1, 2016

Next Release Changelog:

@raamdev
Copy link
Contributor

raamdev commented Mar 2, 2016

Your changelog didn't get automatically added to the release draft, so I'm adding it manually. ↑

@jaswrks
Copy link
Contributor Author

jaswrks commented Mar 2, 2016

Thanks!

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.

2 participants