Separate the connection library into its own package. #12399

zinigor · 2019-05-16T21:37:41Z

This is a work in progress PR created to be able to review, exchange opinions and iterate. Currently it adds a package that would serve as a Connection related library.

Please keep in mind several things as you go through the code:

The interface Manager_Interface has been added as an enforcement measure, I am planning to remove it once all the methods are implemented. It was meant to help to understand what methods will need to be public, and what methods will need to be accessible inside the class.
The naming is different from the one we use in Jetpack and WordPress, I'm happy to hear your thoughts on how you would call things, and where you would put them - this is all still in flux.
The design pattern I'm trying to follow here is to inject all foreign objects using setter methods. We can probably pass them in at initialization time too, please share your concerns regarding this matter too.
Anything you see that is wrong, please do comment. This is all uncharted territory for me, and I'd wager for Jetpack as well.

Testing instructions:

The only testing you can now do is unit testing:
go to packages/jetpack-connection
composer install
composer run test

packages/jetpack-connection/lib/Manager.php

gravityrail · 2019-05-16T23:21:15Z

It's hard for me to comment in detail until I see how this is invoked from the existing code. The class itself seems really clean.

I somewhat prefer injecting implementations using filters rather than setters, or perhaps even referring to classes directly, since that's the WordPress way of doing things. Setters have a way of introducing tons of boilerplate.

So instead of:

// in the invoking class
$connection = new Jetpack_Connection();
$connection->set_option_backend( new Jetpack_Options() );
$connection->set_something_else( new Jetpack_Foo() );

We would do either:

// somewhere in Jetpack_Connection
apply_filters( 'jetpack_options_manager' )->write( 'foo', 'bar' );

or

// somewhere in Jetpack_Connection
(new Jetpack_Options)->write( 'foo', 'bar' ); // this shorthand is PHP 5.4+

Also, as a matter of style, I don't think calling something "foo_backend" is very explanatory. If it's an implementation of an interface, you might call it "foo_impl". But generally, you want the suffix to be a verb or a role. For example:

$options // just the raw name
$options_manager // the role
$options_writer // a verb

enejb · 2019-05-16T23:38:39Z

Nit pick: Could we rename the test folder to tests?

zinigor · 2019-05-17T21:51:27Z

Thanks a lot for your reviews, @gravityrail and @enejb . I have implemented the fixes, the main changes are:

we are now using a constant to determine the option name;
tests are located in the folder named tests;
we assume that WordPress API is available and use filters in the code;
setters are no longer used, managers and callables are set using filters;
we now depend on the WP_Mock library by 10Up here;
we now depend on this library being developed at least in PHP 7.1, which is OK since we will have integration tests in Jetpack for older PHP versions as well.

roccotripaldi · 2019-05-20T17:23:20Z

The class itself seems really clean.

I agree!

Just a random thought: should we start by extracting an XMLRPC package first?

packages/jetpack-connection/lib/Manager.php

jetpackbot · 2019-05-21T11:56:04Z

	Warnings
⚠️	"Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 5be105f

zinigor · 2019-05-21T11:56:57Z

@roccotripaldi thanks for taking a look! Yes, I thought the XMLRPC bit would be also moved into the connection package, because it's a crucial part of the communication mechanism.

enejb · 2019-05-27T21:15:20Z

packages/jetpack-options/tests/Manager.php

+			'args' => array( 'jetpack_uncompact_option_name', false ),
+			'return' => true,
+		) );
+		\WP_Mock::userFunction( 'is_multisite', array(


Instead of setting this us so many times can we just set it once and then overwrite it when we need to?

Yes, I need to make it more DRY, thanks!

tyxla

Really nice work, Igor!

I've left some questions.

tyxla · 2019-05-28T08:39:58Z

packages/jetpack-connection/.gitignore

@@ -0,0 +1,2 @@
+composer.lock


We already have composer.lock ignored in every package:

jetpack/packages/.gitignore

Line 2 in 40d0573

/**/composer.lock

So this shouldn't be necessary.

tyxla · 2019-05-28T08:42:10Z

packages/jetpack-connection/.gitignore

@@ -0,0 +1,2 @@
+composer.lock
+vendor/


Hm, I wouldn't expect us to have to run composer install in every package to make this work inside Jetpack. I think ideally we should just run a single composer install inside the Jetpack root, and that'd get us all dependencies we need.

Even if what I said above isn't true for some reason, would it make sense to move this to the packages/.gitignore file so it would be applicable to all the packages?

We're still deciding on this, but yes, I thought we were going to have dev dependencies in separate packages that will be needed, for example, for running unit tests in isolation.

But whatever we decide, you're right, we can move that up a level to the parent gitignore file.

tyxla · 2019-05-28T08:43:52Z

packages/jetpack-connection/composer.json

+	"require": {},
+	"require-dev": {
+		"phpunit/phpunit": "7.*.*",
+		"10up/wp_mock": "0.4.2"


My concern with WP_Mock is that it would introduce a new way of testing things, so we'd have to refactor all our existing tests to use it. Also, it moves away from the traditional means of writing tests, followed in WP core development. Should we try to stay as close to WP core as possible?

The problem with WP Core tests is that it requires a working WordPress installation, whereas this approach allows us to really isolate the components and run unit tests as they were meant to be run. I don't see a problem with having separate testing frameworks as long as they don't get in our way.

Thanks for elaborating. To be fair, I really love your idea ❤️

However, isn't it too far from us? Our current tests rely on having a working WordPress installation, and many of them actually would need more than just mocking one or two WordPress functions/classes.

Ideally, our plugin and tests would work without WordPress, but we have to be fair that this seems a bit far from what we currently have. And also I think it would cause a challenge for migrating to independent packages, at least initially.

I wonder if we could and should postpone the tests migration to non-WP-Core based ones to another time, when we can focus better on it?

tyxla · 2019-05-28T08:45:22Z

packages/jetpack-connection/composer.json

+	"version": "1.0.0",
+	"require": {},
+	"require-dev": {
+		"phpunit/phpunit": "7.*.*",


My expectation is to have a unified tests package that we could use as a dependency in each of our packages. Then, each package could have its own tests and have them run easily. I'm planning to experiment with that soon, see https://github.com/Automattic/jpop-issues/issues/3889#issuecomment-496414966

In the meantime, does it make sense to not introduce different means of test bootstrapping and running, and have the tests work the same and live in the same place that the existing Jetpack tests do?

The same discussion is better held here, I think:
https://github.com/Automattic/jpop-issues/issues/3889#issuecomment-496495374

tyxla · 2019-05-28T08:45:51Z

packages/jetpack-connection/lib/Manager.php

@@ -0,0 +1,262 @@
+<?php
+
+namespace Jetpack\V7\Connection;


I'm still not convinced about the versioning in the namespace. What's the benefit we're getting with it?

With the new work on the autoloader that @enejb is doing, I think we will be able to let go of versions in namespaces. I'll work on that, thanks for pointing it out!

tyxla · 2019-05-28T08:48:46Z

packages/jetpack-connection/lib/Manager.php

+	 *
+	 * @param Integer $user_id the user identifier.
+	 */
+	public static function disconnect_user( $user_id ) {


I wonder if we can totally get rid of any static methods in the new implementation. Not necessary for the first pass, of course, just wanted to hear your thoughts.

Yes, I agree, this was added in the early stages when I wasn't yet sure how namespaced classes and functions would work. I agree that when using objects, we should rely on instance methods as much as possible.

tyxla · 2019-05-28T08:49:17Z

packages/jetpack-connection/lib/Manager.php

+	public function generate_secrets( $action, $user_id, $exp ) {
+		$callable = $this->get_secret_callable();
+
+		$secret_name = 'jetpack_' . $action . '_' . $user_id;


Suggested change

$secret_name = 'jetpack_' . $action . '_' . $user_id;

$secret_name = 'jetpack_' . $action . '_' . $user_id;

tyxla · 2019-05-28T08:50:41Z

packages/jetpack-connection/lib/Manager.php

+			 *
+			 * @param Jetpack_Options an option manager object.
+			 */
+			$this->option_manager = apply_filters( 'jetpack_connection_option_manager', false );


I found it unexpected that a get_something method actually sets that something. Should we have the corresponding set_option_manager method defined and called in the right time so the dependency injection is done more visibly?

Yes, this has been discussed earlier here: #12399 (comment)
And we decided to go the other way to stay closer to the way WordPress uses filters.

Lovely, thanks for elaborating 👍 I like that approach very much :)

tyxla · 2019-05-28T08:55:22Z

packages/jetpack-connection/lib/Manager.php

+			 *
+			 * @param Jetpack_Options an option manager object.
+			 */
+			$this->option_manager = apply_filters( 'jetpack_connection_option_manager', false );


Can we end up running (false)->update_option() and (false)->get_option() if we default to false for option manager? Should we default to new Jetpack\V7\Options\Manager or a new object of its inheriting class (just noticed that's this one is abstract).

Yes, there needs to be a sane default or better error handling in case the manager is not set. I don't think there's going to be a case when this happens at runtime, though - it's a matter of an improperly set up class.

tyxla · 2019-05-28T08:56:05Z

packages/jetpack-options/lib/Manager.php

@@ -0,0 +1,243 @@
+<?php


Could we use /src instead of /lib?

Sure thing, let's stick to one schema everywhere, thanks!

matticbot · 2019-05-30T08:50:51Z

Caution: This PR has changes that must be merged to WordPress.com
Hello zinigor! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D28891-code before merging this PR. Thank you!

matticbot · 2019-05-30T14:22:41Z

zinigor, Your synced wpcom patch D28891-code has been updated.

gravityrail · 2019-05-30T17:51:08Z

Drive-by comment: I think we decided to prefix packages with Automattic\Jetpack, right?

matticbot · 2019-05-30T21:09:43Z

zinigor, Your synced wpcom patch D28891-code has been updated.

enejb

I think in the Options_Manager class we should avoid using the word option. Since that can be implied.

enejb · 2019-05-30T21:16:20Z

packages/jetpack-options/src/Manager.php

Can we just use get instead of get_option ?

enejb · 2019-05-30T21:17:17Z

packages/jetpack-options/src/Manager.php

Can we make this private? Should it be called outside this class?

enejb · 2019-05-30T21:17:36Z

packages/jetpack-options/src/Manager.php

Can we call this just update?

enejb · 2019-05-30T21:17:56Z

packages/jetpack-options/src/Manager.php

Can we make this private as well?

enejb · 2019-05-30T21:18:21Z

packages/jetpack-options/src/Manager.php

Can we just this just delete?

enejb · 2019-05-30T21:19:12Z

packages/jetpack-options/src/Manager.php

can we make this just is_network

matticbot · 2019-05-31T12:20:25Z

zinigor, Your synced wpcom patch D28891-code has been updated.

matticbot · 2019-05-31T21:33:36Z

zinigor, Your synced wpcom patch D28891-code has been updated.

zinigor · 2019-05-31T22:06:57Z

Just wanted to mention again here that the Connection Manager Interface that I have added was only intended as a tool to enforce method creation. I wanted to remove the interface in the end, and if you feel like things can be done differently, please feel free to change whatever you need. Especially in light of Enej's review regarding naming - now is probably the best way to agree on the best naming strategy.

One more thing: I have tried to fix tests in 5e2c656, but for some reason the files were not properly excluded. Tests in D28891-code were properly excluded, but the latest changes in this PR have overwritten the diff, please look at the diff history to see what I did to make the tests pass.

…-add them later.

roccotripaldi · 2019-06-03T16:48:55Z

I'm going to merge this, though it's not fully done yet.

Still to do:

Make Jetpack options a true package Jetpack DNA: Options Package #12537
Make an XMLRPC package: Jetpack DNA: XMLRPC package #12538
Make adjustments within the Connection package so that it actually works: Jetpack DNA: Connection package update #12539

@enejb

* Add Custom Autoloader (#12447) * Try Custom Autoloader * spelling fix Co-Authored-By: Marin Atanasov <[email protected]> * minor space fix Co-Authored-By: Marin Atanasov <[email protected]> * minor space fix. Co-Authored-By: Marin Atanasov <[email protected]> * Updated to make it even simpler * test plugin * a Toy plugin :) 🚂 * Minor update * Minor update * Fixes typo * delete file * Minor updates * Added limitaionts section to readme * ooops * Fix readme * Turn the jetpack-autoloader into a package. * fixes to the jetpack a custom plugin * typo Co-Authored-By: Marin Atanasov <[email protected]> * remove scripts to "a" no longer needed thanks to @enejb magic * Update packages/autoloader/README.md fixes more wording in the readme Co-Authored-By: Miguel Lezama <[email protected]> * Update the version numbers of packages insead of branches * forgot about the plugin * update "a" composer * Update the a composer lock file. * Revert "Update the a composer lock file." This reverts commit 90f2cb8dbc50e15caf19fa0b88e1ee201f8f21c5. * typo fix Co-Authored-By: Miguel Lezama <[email protected]> * Rebased against master, testes that a plugin works as expected * Remove not needed code * Minor readme fix * Remoce Space * Add filter to prevent the autoloader from loading a particular version * add b toy plugin depending on Jetpack Logo 1.0.1 Co-Authored-By: Marin Atanasov <[email protected]> * minor space fix Co-Authored-By: Miguel Lezama <[email protected]> * Update a comment * Fix phpcs fixes * Make the remove class_map from the global name space, fix phpcs issues * Add the autoload_packages to be ignored. * Add debug info * Remove versions from composer.json of packages * Add missing vendor to namespace * Fix typo * Fix typo and add punctuation * Fix a typo * Update .gitignore Co-Authored-By: Jeremy Herve <[email protected]> * Update all the composer files * Take into account the @dev and always use the dev-* version * Bug Fix load the latest version! * Update the logic when enqueing the classes * Update to use Namespaces * Add tests to make sure that the enqueing works as expected * Fix suggestions from Marin * Remove generateClassMap method * Fix the tests, rename the fucntion name to be more accurate * More fixes to the tests. * FuN * Making it all work again * Update the lock file * Minor * Remove unused plugins (#12529) * Removed unnecessary gitignore lines. * Separate the connection library into its own package. (#12399) * Added an interface as a guide for the new package methods. * Add package structure * Initialized the composer library, added gitignore. * Removed phpunit from connection dependencies. * Added first real code to the connection manager class, as well as tests for it. * Renamed the test folder to tests. * Made the option name be a class constant. * Added 10up's WP_Mock as a dependency, refactored the injection to use filters instead of setters. * Changed the internal method accessibility to protected. * Initialized an empty package repo for the options manager. * Added the options package as a dependency to the master repo. * Added a package structure placeholder for the options manager. * Added the first test for the delete option method. * Added the update method and a test for it. * Added a test for the grouped option deletion. * Added the group updater method and a test for it. * Added the grouped option getter and a test for it. * Added a test for the noncompact option getter. * Added tests for network option manipulation. * Removed autoloader requirement in favour of the existing one. * Added the connection manager secret generator to actual Jetpack class. * Removed individual gitignore files in favor of a parent-level file, props @tyxla! * Moved packages source code into src folders. * Removed the V7 part of the namespace. * Remove v7 from namespace in the Manager Interface * Remove V7 from namespaces * Add Automattic to the namespace * Rename jetpack-[name] packages to [name] * Update composer lock * Adding a package for Jetpack Constants * Attempting to fix WPCOM tests. * Excluding packages folder from linting for PHP 5.6 and earlier. * adjust syntax in connection tests * removing new unit tests. i'm not sure why they are failing. we can re-add them later. * reverting last commit, and fixing tests in `packages/options` * ah, one more * remove an un-needed linting hack (#12543) * Composer: avoid duplicate repositories key (#12546) * Composer: avoid duplicate repositories key Rely on wildcard instead. See #12483 * Revert changes in composer.lock * Revert changes in composer.lock * Revert changes in composer.lock * Packages: Make logo package tests independent (#12514) * Move package tests to package directory * Add some dev deps to logo package * Ignore vendor of all packages * Add a logo package simple bootstrap * Add a logo package phpunit config * Cleanup package tests from Jetpack phpunit config * Remove WP prefix from class name * Make sure composer is installed * Try: add SCOPE to travis matrix * Try: Disable older PHP versions * Replace WP_Mock with PHPMock * Enable php 7.0 in Travis again * Make phpunit version more permissive * Fix setUp signature * Play with phpunit versions * Try enabling CI for feature branch * Try enabling CI for feature branch * Try enabling CI for feature branch * Revert latest changes * Try adding the branch to the whitelist * Enable PHP 5.6 again * Use php-mock instead of php-mock-phpunit * Run composer phpunit for all packages * Make mocking method protected * Add parent teardown * Remove whitelisted branch * Run only for packages with tests * remove trailing slash 🤔 * nope * not sure what I am doing * add debug statements * run only if we have `php` tests @zinigor @tyxla we need to normalise de connection one * Jetpack Options Package: just class map existing options class (#12562) * instead of a new implementation of jetpack options, let's class map it * Packages: Fix connection tests and allow them to run in CI (#12560) * Use php-mock instead of wp_mock * Add phpunit script * Migrate tests from wp_mock to phpmock * Make tests run in CI * Run travis only for packages that have the php test dir * Revert "Jetpack Options Package: just class map existing options class (#12562)" (#12571) This reverts commit 0e3ac7a. * Packages: Finish the constants package (#12558) * PSR-4 constants * Actually add namespacing to the constants class * Update composer.lock * Fix phpcs errors and add some docs * Add a package README * Move constant tests to the package * Update Jetpack to use the constants package * Fix constants usage in options package * Rename README variable Co-Authored-By: Miguel Lezama <[email protected]> * Fix typo Co-Authored-By: Miguel Lezama <[email protected]> * Use plural Co-Authored-By: Miguel Lezama <[email protected]> * Making Jetpack_Options a classmapped package. (#12577) * Making Jetpack_Options a classmapped package. * Update Jetpack to use new JITM package (#12516) * Update Jetpack to use new JITM package * Add instructions on package installation and usage (#12576) * Add instructions on package installation and usage * Move content about installing Composer to the doc regarding development environment. Add a pointer to that section in the packages' readme * Explain how to install Composer on systems other than macOS. Props @aldavigdis * Update docs/development-environment.md Co-Authored-By: Jeremy Herve <[email protected]> * Apply suggestions from code review Co-Authored-By: Jeremy Herve <[email protected]> * Add reference to Composer installation in the section that explains how to start developing for Jetpack * Packages: Fix alias example in README (#12582) * Packages: Move sync to a classmapped package (#12572) * Move all sync files to the package * Add sync package composer.json * Composer require the new sync package * Update composer.lock after adding sync package dependency * Remove some sync file requiring in sync package * Remove some sync file requiring in the rest of Jetpack * Hook Jetpack actions properly * Fix a rebase conflict * Fix update post terms test to not compare against the DB * Packages: Move JITM tests to package and fix deps (#12583) * Add phpunit devdep and composer phpunit script * Add JITM tests config * Move tests and add bootstrap * Remove legacy jitm suite * Rely on @dev connection package * Remove version * Update minimum stability and prefer stable * Fix tests * Update composer.lock * Fix whitespace Co-Authored-By: Miguel Lezama <[email protected]> * Prepend an empty line Co-Authored-By: Miguel Lezama <[email protected]> * Connection Package: add Jetpack_Data methods (#12580) Jetpack_Data -> Connection package * removes php_bug_66229_check method - it's no longer used in the code base. * Moving is_usable_domain to Connection package. * Moves `get_access_token` to Connection package. * Create package for Jetpack Tracking (#12596) Created package for Jetpack Tracking * Update/package logo add gray (#12610) * Update the logo package to include a gray version. Add parameter to render method to specify the type * Remove svg files. Update method to return an SVG string that can be stylized with CSS. Update tests. * Fixed issue with logo in Jetpack connection banner in WP Admin * Update test so it strictly tests two instances of the CSS class * Optimize performance. Use const since the string is immutable. Use nowdoc to compose the string. * Update logo tests in new file * Constants_Manager -> Constants * inc/lib/admin-pages/class.jetpack-react-page.php (#12627) * Add jetpack signature class to connection package (#12620) * Connection package: Add class.jetpack-signature and class.jetpack-error to connection package * DNA: Don't require composer autoloader (#12624) * Jitm package, don't use global Jetpack class (#12629)

Those were already removed in #12479, but were added back by mistake in #12399

@enejb

* Add Custom Autoloader (#12447) * Try Custom Autoloader * spelling fix Co-Authored-By: Marin Atanasov <[email protected]> * minor space fix Co-Authored-By: Marin Atanasov <[email protected]> * minor space fix. Co-Authored-By: Marin Atanasov <[email protected]> * Updated to make it even simpler * test plugin * a Toy plugin :) 🚂 * Minor update * Minor update * Fixes typo * delete file * Minor updates * Added limitaionts section to readme * ooops * Fix readme * Turn the jetpack-autoloader into a package. * fixes to the jetpack a custom plugin * typo Co-Authored-By: Marin Atanasov <[email protected]> * remove scripts to "a" no longer needed thanks to @enejb magic * Update packages/autoloader/README.md fixes more wording in the readme Co-Authored-By: Miguel Lezama <[email protected]> * Update the version numbers of packages insead of branches * forgot about the plugin * update "a" composer * Update the a composer lock file. * Revert "Update the a composer lock file." This reverts commit 90f2cb8dbc50e15caf19fa0b88e1ee201f8f21c5. * typo fix Co-Authored-By: Miguel Lezama <[email protected]> * Rebased against master, testes that a plugin works as expected * Remove not needed code * Minor readme fix * Remoce Space * Add filter to prevent the autoloader from loading a particular version * add b toy plugin depending on Jetpack Logo 1.0.1 Co-Authored-By: Marin Atanasov <[email protected]> * minor space fix Co-Authored-By: Miguel Lezama <[email protected]> * Update a comment * Fix phpcs fixes * Make the remove class_map from the global name space, fix phpcs issues * Add the autoload_packages to be ignored. * Add debug info * Remove versions from composer.json of packages * Add missing vendor to namespace * Fix typo * Fix typo and add punctuation * Fix a typo * Update .gitignore Co-Authored-By: Jeremy Herve <[email protected]> * Update all the composer files * Take into account the @dev and always use the dev-* version * Bug Fix load the latest version! * Update the logic when enqueing the classes * Update to use Namespaces * Add tests to make sure that the enqueing works as expected * Fix suggestions from Marin * Remove generateClassMap method * Fix the tests, rename the fucntion name to be more accurate * More fixes to the tests. * FuN * Making it all work again * Update the lock file * Minor * Remove unused plugins (#12529) * Removed unnecessary gitignore lines. * Separate the connection library into its own package. (#12399) * Added an interface as a guide for the new package methods. * Add package structure * Initialized the composer library, added gitignore. * Removed phpunit from connection dependencies. * Added first real code to the connection manager class, as well as tests for it. * Renamed the test folder to tests. * Made the option name be a class constant. * Added 10up's WP_Mock as a dependency, refactored the injection to use filters instead of setters. * Changed the internal method accessibility to protected. * Initialized an empty package repo for the options manager. * Added the options package as a dependency to the master repo. * Added a package structure placeholder for the options manager. * Added the first test for the delete option method. * Added the update method and a test for it. * Added a test for the grouped option deletion. * Added the group updater method and a test for it. * Added the grouped option getter and a test for it. * Added a test for the noncompact option getter. * Added tests for network option manipulation. * Removed autoloader requirement in favour of the existing one. * Added the connection manager secret generator to actual Jetpack class. * Removed individual gitignore files in favor of a parent-level file, props @tyxla! * Moved packages source code into src folders. * Removed the V7 part of the namespace. * Remove v7 from namespace in the Manager Interface * Remove V7 from namespaces * Add Automattic to the namespace * Rename jetpack-[name] packages to [name] * Update composer lock * Adding a package for Jetpack Constants * Attempting to fix WPCOM tests. * Excluding packages folder from linting for PHP 5.6 and earlier. * adjust syntax in connection tests * removing new unit tests. i'm not sure why they are failing. we can re-add them later. * reverting last commit, and fixing tests in `packages/options` * ah, one more * remove an un-needed linting hack (#12543) * Composer: avoid duplicate repositories key (#12546) * Composer: avoid duplicate repositories key Rely on wildcard instead. See #12483 * Revert changes in composer.lock * Revert changes in composer.lock * Revert changes in composer.lock * Packages: Make logo package tests independent (#12514) * Move package tests to package directory * Add some dev deps to logo package * Ignore vendor of all packages * Add a logo package simple bootstrap * Add a logo package phpunit config * Cleanup package tests from Jetpack phpunit config * Remove WP prefix from class name * Make sure composer is installed * Try: add SCOPE to travis matrix * Try: Disable older PHP versions * Replace WP_Mock with PHPMock * Enable php 7.0 in Travis again * Make phpunit version more permissive * Fix setUp signature * Play with phpunit versions * Try enabling CI for feature branch * Try enabling CI for feature branch * Try enabling CI for feature branch * Revert latest changes * Try adding the branch to the whitelist * Enable PHP 5.6 again * Use php-mock instead of php-mock-phpunit * Run composer phpunit for all packages * Make mocking method protected * Add parent teardown * Remove whitelisted branch * Run only for packages with tests * remove trailing slash 🤔 * nope * not sure what I am doing * add debug statements * run only if we have `php` tests @zinigor @tyxla we need to normalise de connection one * Jetpack Options Package: just class map existing options class (#12562) * instead of a new implementation of jetpack options, let's class map it * Packages: Fix connection tests and allow them to run in CI (#12560) * Use php-mock instead of wp_mock * Add phpunit script * Migrate tests from wp_mock to phpmock * Make tests run in CI * Run travis only for packages that have the php test dir * Revert "Jetpack Options Package: just class map existing options class (#12562)" (#12571) This reverts commit 0e3ac7a. * Packages: Finish the constants package (#12558) * PSR-4 constants * Actually add namespacing to the constants class * Update composer.lock * Fix phpcs errors and add some docs * Add a package README * Move constant tests to the package * Update Jetpack to use the constants package * Fix constants usage in options package * Rename README variable Co-Authored-By: Miguel Lezama <[email protected]> * Fix typo Co-Authored-By: Miguel Lezama <[email protected]> * Use plural Co-Authored-By: Miguel Lezama <[email protected]> * Making Jetpack_Options a classmapped package. (#12577) * Making Jetpack_Options a classmapped package. * Update Jetpack to use new JITM package (#12516) * Update Jetpack to use new JITM package * Add instructions on package installation and usage (#12576) * Add instructions on package installation and usage * Move content about installing Composer to the doc regarding development environment. Add a pointer to that section in the packages' readme * Explain how to install Composer on systems other than macOS. Props @aldavigdis * Update docs/development-environment.md Co-Authored-By: Jeremy Herve <[email protected]> * Apply suggestions from code review Co-Authored-By: Jeremy Herve <[email protected]> * Add reference to Composer installation in the section that explains how to start developing for Jetpack * Packages: Fix alias example in README (#12582) * Packages: Move sync to a classmapped package (#12572) * Move all sync files to the package * Add sync package composer.json * Composer require the new sync package * Update composer.lock after adding sync package dependency * Remove some sync file requiring in sync package * Remove some sync file requiring in the rest of Jetpack * Hook Jetpack actions properly * Fix a rebase conflict * Fix update post terms test to not compare against the DB * Packages: Move JITM tests to package and fix deps (#12583) * Add phpunit devdep and composer phpunit script * Add JITM tests config * Move tests and add bootstrap * Remove legacy jitm suite * Rely on @dev connection package * Remove version * Update minimum stability and prefer stable * Fix tests * Update composer.lock * Fix whitespace Co-Authored-By: Miguel Lezama <[email protected]> * Prepend an empty line Co-Authored-By: Miguel Lezama <[email protected]> * Connection Package: add Jetpack_Data methods (#12580) Jetpack_Data -> Connection package * removes php_bug_66229_check method - it's no longer used in the code base. * Moving is_usable_domain to Connection package. * Moves `get_access_token` to Connection package. * Create package for Jetpack Tracking (#12596) Created package for Jetpack Tracking * Update/package logo add gray (#12610) * Update the logo package to include a gray version. Add parameter to render method to specify the type * Remove svg files. Update method to return an SVG string that can be stylized with CSS. Update tests. * Fixed issue with logo in Jetpack connection banner in WP Admin * Update test so it strictly tests two instances of the CSS class * Optimize performance. Use const since the string is immutable. Use nowdoc to compose the string. * Update logo tests in new file * Constants_Manager -> Constants * inc/lib/admin-pages/class.jetpack-react-page.php (#12627) * Add jetpack signature class to connection package (#12620) * Connection package: Add class.jetpack-signature and class.jetpack-error to connection package * DNA: Don't require composer autoloader (#12624) * Jitm package, don't use global Jetpack class (#12629)

zinigor added [Status] In Progress DO NOT MERGE don't merge it! [Focus] Jetpack DNA labels May 16, 2019

zinigor requested a review from a team May 16, 2019 21:37

enejb reviewed May 16, 2019

View reviewed changes

packages/jetpack-connection/lib/Manager.php Outdated Show resolved Hide resolved

gravityrail reviewed May 20, 2019

View reviewed changes

packages/jetpack-connection/lib/Manager.php Outdated Show resolved Hide resolved

enejb reviewed May 27, 2019

View reviewed changes

tyxla reviewed May 28, 2019

View reviewed changes

matticbot added the Touches WP.com Files label May 30, 2019

enejb reviewed May 30, 2019

View reviewed changes

jeherve added the Connect Flow Connection banners, buttons, ... label May 31, 2019

roccotripaldi changed the base branch from master to feature/jetpack-packages May 31, 2019 19:58

dereksmart force-pushed the feature/jetpack-packages branch from 7a4d3f3 to 7df90c1 Compare May 31, 2019 20:27

gravityrail added [Status] Needs Review This PR is ready for review. and removed DO NOT MERGE don't merge it! labels May 31, 2019

zinigor and others added 4 commits May 31, 2019 17:24

Added an interface as a guide for the new package methods.

876790b

Add package structure

4a6db30

Initialized the composer library, added gitignore.

f13489f

Removed phpunit from connection dependencies.

d2e74b1

gravityrail and others added 6 commits May 31, 2019 17:31

Add Automattic to the namespace

b1066c9

Rename jetpack-[name] packages to [name]

1288f29

Update composer lock

09a0774

Adding a package for Jetpack Constants

afa5096

Attempting to fix WPCOM tests.

57ea581

Excluding packages folder from linting for PHP 5.6 and earlier.

5e2c656

roccotripaldi force-pushed the try/connection-split branch from b4f32c0 to 5e2c656 Compare May 31, 2019 21:33

lezama mentioned this pull request May 31, 2019

Sync: Move to an independent package #12524

Closed

roccotripaldi added 4 commits June 3, 2019 11:23

adjust syntax in connection tests

1b9bb2a

removing new unit tests. i'm not sure why they are failing. we can re…

98ba081

…-add them later.

reverting last commit, and fixing tests in packages/options

56aedcc

ah, one more

5be105f

This was referenced Jun 3, 2019

Jetpack DNA: Options Package #12537

Closed

Jetpack DNA: Connection package update #12539

Closed

roccotripaldi merged commit a371897 into feature/jetpack-packages Jun 3, 2019

matticbot added [Status] Needs Changelog and removed [Status] Needs Review This PR is ready for review. labels Jun 3, 2019

dereksmart deleted the try/connection-split branch June 3, 2019 16:50

roccotripaldi mentioned this pull request Jun 3, 2019

remove an un-needed linting hack #12543

Merged

jeherve added this to the 7.5 milestone Jun 3, 2019

jeherve removed [Status] Needs Changelog [Status] In Progress labels Jun 3, 2019

jeherve added a commit that referenced this pull request Jun 21, 2019

Remove Composer installers

9ecf160

Those were already removed in #12479, but were added back by mistake in #12399

jeherve added a commit that referenced this pull request Jun 21, 2019

Remove Composer installers (#12807)

d7250a1

Those were already removed in #12479, but were added back by mistake in #12399

jeherve added a commit that referenced this pull request Jun 21, 2019

Remove Composer installers (#12807)

4a55a2a

Those were already removed in #12479, but were added back by mistake in #12399

	$secret_name = 'jetpack_' . $action . '_' . $user_id;
	$secret_name = 'jetpack_' . $action . '_' . $user_id;

Separate the connection library into its own package. #12399

Separate the connection library into its own package. #12399

Uh oh!

Conversation

zinigor commented May 16, 2019

Testing instructions:

Uh oh!

Uh oh!

gravityrail commented May 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enejb commented May 16, 2019

Uh oh!

zinigor commented May 17, 2019

Uh oh!

roccotripaldi commented May 20, 2019

Uh oh!

Uh oh!

jetpackbot commented May 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zinigor commented May 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyxla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matticbot commented May 30, 2019

Uh oh!

matticbot commented May 30, 2019

Uh oh!

gravityrail commented May 30, 2019

Uh oh!

matticbot commented May 30, 2019

Uh oh!

enejb left a comment

Choose a reason for hiding this comment

gravityrail commented May 16, 2019 •

edited

Loading

jetpackbot commented May 21, 2019 •

edited

Loading