Skip to content

Removed duplicate self-executing anonymous function#12184

Merged
jeherve merged 1 commit intoAutomattic:masterfrom
mahdiyazdani:patch-1
May 9, 2019
Merged

Removed duplicate self-executing anonymous function#12184
jeherve merged 1 commit intoAutomattic:masterfrom
mahdiyazdani:patch-1

Conversation

@mahdiyazdani
Copy link
Copy Markdown
Contributor

@mahdiyazdani mahdiyazdani commented Apr 28, 2019 •
edited by jeherve
Loading

By default wc_enqueue_js method comes with a built-in self-executing anonymous function which makes it unnecessary to write our own within the code.

Reference can be found here.

Testing Instructions

1- Upgrade to Jetpack premium
2- Enter a GA Tracking code
3- Enable basic tracking which uses legacy ga.js to track
4- Visit the front-end and inspect the source to find the tracking code
5- See the duplication of the anonymous function

Proposed Changelog entry

  • WooCommerce Analytics: remove duplicate self-executing anonymous function.

@mahdiyazdani
Removed duplicate self-executing anonymous function
9042ca0 
By default `wc_enqueue_js` method comes with a built-in self-executing anonymous function which makes it unnecessary to write our own within the code.

Reference can be found [here](https://github.com/woocommerce/woocommerce/blob/9206d6f4dd55df6cc45ea3982656cc8ec7690a76/includes/wc-core-functions.php#L859).
@mahdiyazdani mahdiyazdani requested a review from a team April 28, 2019 11:12
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Apr 28, 2019

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review
⚠️ "Testing instructions" are missing for this PR. Please add some
⚠️ "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 9042ca0

@jeherve jeherve added Bug When a feature is broken and / or not performing as intended [Feature] WooCommerce Analytics [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Apr 29, 2019
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Apr 29, 2019

Thanks for the contribution. Could you add some testing instructions to your PR description, to make your PR easier to review?

Thank you.

@kraftbj kraftbj added this to the 7.4 milestone Apr 29, 2019
@mahdiyazdani
Copy link
Copy Markdown
Contributor Author

mahdiyazdani commented May 5, 2019 •
edited
Loading

@jeherve The PR removes the duplication of the following anonymous function from the front-end.

jQuery( function( $ ) {} );

To test:

1- Upgrade to JetPack premium :)
2- Enter a GA Tracking code
3- Enable basic tracking which uses legacy ga.js to track
4- Visit the front-end and inspect the source to find the tracking code
5- See the duplication of the anonymous function

@jeherve jeherve added [Status] Needs Review This PR is ready for review. [Type] Janitorial and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels May 6, 2019
@jeherve
Copy link
Copy Markdown
Member

jeherve commented May 6, 2019

Thank you. I edited your PR description to add all this.

jeherve
jeherve approved these changes May 9, 2019
View reviewed changes
Copy link
Copy Markdown
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.

This seems okay to me. 👍

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels May 9, 2019
@jeherve jeherve merged commit 3ffb42c into Automattic:master May 9, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 9, 2019
jeherve added a commit that referenced this pull request May 17, 2019
@jeherve
changelog: add #12184
8ec49ff
jeherve added a commit that referenced this pull request May 23, 2019
@jeherve
changelog: add #12184
00db37e
jeherve added a commit that referenced this pull request May 27, 2019
@kraftbj @jeherve
7.4 Changelog (#12304)
0fb4436 
* Kick off the changelog

* Add 7.3.1

* Update date and post link

* changelog: add #12219

* changelog: add #12170

* changelog: add #12184

* Changelog: add #12268

* Changelog: add #12081

* Changelog: add #12323

* Changelog: add #12204

* Changelog: add #12269

* Changelog: add #12332

* changelog: add #12339

* changelog: add #12209

* Changelog: add #12319

* Changelog: add #12357

* Changelog: add #12124

* Changelog: add #12373

* Changelog: add #12252

* Changelog: add #12383

* Changelog: add #12372

* changelog: add #12337

* Changelog: add #12290

* Changelog: add #12301

* Changelog: add #12061

* Testing list: add instructions for #12061

* Changelog: add #12393

* Update minimum supported version

See #12287

* Changelog: add #12406

* Testing list: add #12406

* Changelog: add #12277

* Changelog: add #12412

* Changelog: add #11318

* Changelog: add #12328

* Changelog: add #12425

* Changelog: add #12380

* Changelog: add #12428

* Changelog: add #12414

* Changelog: add #12395

* Changelog & Testing list: add #12416, #12417, #12418, and #12348

* changelog: add #12379

* Changelog: add #12341

* changelog: add #12444

* Changelog: add #12434

* Changelog: add #12454

* Changelog: add #12460

* Changelog: add #12463

* Changelog: add #12457

* Changelog / testing list: add #10333

* Changelog: add #12467


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

No one assigned

Labels

Bug When a feature is broken and / or not performing as intended [Feature] WooCommerce Analytics [Type] Janitorial

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

5 participants

@mahdiyazdani @jetpackbot @jeherve @kraftbj @matticbot