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

Widgets: Search: Unify Jetpack and WP.com Code #10084

Merged
merged 1 commit into from
Sep 7, 2018
Merged

Conversation

mdawaffe
Copy link
Member

Changes proposed in this Pull Request:

  • Extract default name into constructor.
  • Extract search activation into methods. (Making it easy for WordPress.com
    to implement its own methods.)
  • Fix some typos.

Testing instructions:

  1. Create a site with Jetpack Professional plan
  2. Go to https://wordpress.com/settings/traffic/{your site}
  3. Make sure Jetpack Search (bottom of page) is disabled
  4. Go to wp-admin/widgets.php
  5. Add the "Search (Jetpack)" widget.
  6. Go back to https://wordpress.com/settings/traffic/{your site} (reload the page)
  7. (Wait a second or two)
  8. See that Jetpack Search is enabled.

Proposed changelog entry for your changes:

Minor Changes to Jetpack Search Widget

Extract default name into constructor.

Extract search activation into methods. (Making it easy for WordPress.com
to implement its own methods.)
@mdawaffe mdawaffe requested a review from gibrown August 31, 2018 05:44
@mdawaffe mdawaffe requested a review from a team as a code owner August 31, 2018 05:44
@mdawaffe mdawaffe changed the title Widgets: Search: Unify Jetapck and WP.com Code Widgets: Search: Unify Jetpack and WP.com Code Aug 31, 2018
@jetpackbot
Copy link
Collaborator

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is 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

@mdawaffe mdawaffe added [Feature] Extra Sidebar Widgets [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Feature] Search For all things related to Search labels Aug 31, 2018
Copy link
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

WFM and LGTM

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Looking good, working well. Merging.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 7, 2018
@jeherve jeherve merged commit 97f2a54 into master Sep 7, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Sep 7, 2018
@jeherve jeherve deleted the unify/search-widget branch September 7, 2018 12:08
jeherve added a commit that referenced this pull request Sep 14, 2018
jeherve added a commit that referenced this pull request Sep 24, 2018
jeherve added a commit that referenced this pull request Sep 25, 2018
* Readme: add boilerplate for next release, 6.6

* Add 6.5 to the changelog.txt file

* Set boilerplate testing list for 6.6

* Readme: update stable tag to 6.5

* Add bullets to 6.5 changelog items

* Readme: add link to previous changelogs

This will help folks who want to know more about past releases,
while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release.

* Changelog: add information at the top of the changelog file.

* Changelog: add #10054

* Changelog: add #10078

* Changelog: add #10079

* Changelog: add #10064

* Changelog: add #10094

* Changelog: add #10096

* Testing list: add more information based on #10087

* Changelog: add #9847

* Changelog: add #10084

* Changelog: add #9918

* Changelog: add #7614

* Changelog: add #10116

* Changelog: add #10108

* Changelog: add #10041

* Changelog: add #10121

* Changelog: add #10134

* Changelog: add #10130

* Changelog: add #10109

* changelog: add #10137

* changelog: add #9952

* changelog: add #10120

* changelog: add #10162

* Changelog: add #10163

* Changelog: add #10092

* changelog: add #10156

* Changelog: add #10154

* changelog: add #10122

* Changelog: add #10101

* changelog: add #10105

* changelog: add #10190

* Changelog: add #10196

* changelog: add #10152

* Changelog: add #10153

* Testing list: add more details to Site Verification testing steps.

@see #10143 (comment)

* changelog: add #10194

* Changelog: add #10193
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.

5 participants