-
Notifications
You must be signed in to change notification settings - Fork 823
Remove sending is_singular_post parameter #44186
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
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 🔴 Action required: We would recommend that you add a section to the PR description to specify whether this PR includes any changes to data or privacy, like so:
Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 1 file.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good.
I tested it and do not see the is_singular_post
in the adflow/conf/
call. I also verified it by checking the stable version of Jetpack, and we sent it in that one.
I can also see the debug ad placement, including the inline ads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested in localhost with the other branch in my sandbox. works as described and code changes LGTM
Related to ADS-295 and
187107-ghe-Automattic/wpcom
Proposed changes:
is_singular_post
to highlight the current URL is suitable for inline ads (becauseOther information:
Testing instructions:
If you need to know more about how to get Jetpack tests site up and running, please take a look at
184618-ghe-Automattic/wpcom
. Also, please don't forget to make your site launched and be public (use WP Admin > General > Reading section).187107-ghe-Automattic/wpcom
)remove/sending-is-singular-post
branch for Jetpack;is_singular_post
query parameter, in my case the request is?ad_dev=true
)Troubleshooting
In the case you don't see ads due to
af.pubmine.com/?api_version=2
returns 204 (the ad slot considered as Brand Unsafe), just switch to local AdFlow in WATL and run AdFlow locally.