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

Merge REST API into core. #27100

Merged
merged 284 commits into from Aug 6, 2020
Merged

Conversation

vedanshujain
Copy link
Contributor

@vedanshujain vedanshujain commented Jul 22, 2020

All Submissions:

How to review this PR?

This PR contains all commits from release/1.0 branch from REST API repo so its not practical to review directly.

I would recommend heading over to vedanshujain#2, <- this PR contains changes that are added on top of merging from release/1.0 to make sure that API works. Edit: There is also a comment below which documents the merge process to preserve original commit author and details.

I am planning to target 4.5 for this PR, but an early review and merge would be very much appreciated.

Changes proposed in this Pull Request:

Merge API repo's release/1.0 into core.

How to test the changes in this Pull Request:

From vedanshujain#2:

  1. Checkout this branch and wc-rest-api plugin (https://github.com/woocommerce/woocommerce-rest-api/). Disable the REST API plugin for now. Run composer update.
  2. Go to WooCommerce > Status page and look for WooCommerce REST API package setting string. Make sure that its equal to current WC version.
  3. Enable the REST API plugin and refresh the status page. It should now show the API Package version since that;s now loaded instead of API code.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Dev - Merge API Package into core.

@rrennick
Copy link
Contributor

@vedanshujain When I have the REST API plugin action (1.1.0) the SSR shows the core REST API as active instead of the plugin.

@vedanshujain
Copy link
Contributor Author

@vedanshujain When I have the REST API plugin action (1.1.0) the SSR shows the core REST API as active instead of the plugin.

Oversight on my part, should be fixed now.

Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

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

Looks great now!

Copy link
Contributor

@ObliviousHarmony ObliviousHarmony left a comment

Choose a reason for hiding this comment

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

Great job getting this merged back into Core! I also went a step further and tested the API too and everything seems in order.

It might be worth thinking about where these PSR-4 named files like Server, Package, and the Utilities folder should live. Personally I'd like to refactor them but I think they're probably "public API" and should be treated as such since they were never shipped as anything other than part of Core. With that in mind maybe they should be in src since they're namespaced? They can be moved later though so there's nothing to worry about for this PR I think.

Copy link
Contributor

@jonathansadowski jonathansadowski left a comment

Choose a reason for hiding this comment

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

Looks good. It'll be nice to have this merged back to core!

@vedanshujain vedanshujain merged commit 6ba8af1 into woocommerce:master Aug 6, 2020
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Aug 6, 2020
@juliaamosova juliaamosova removed the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: add changelog Mark all PRs that have not had their changelog entries added. [auto]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet