Skip to content

Ads: Fix for wp jetpack warning that HTTP_HOST isn't set.#5874

Merged
dereksmart merged 1 commit intomasterfrom
fix/wordads-undefined
Dec 16, 2016
Merged

Ads: Fix for wp jetpack warning that HTTP_HOST isn't set.#5874
dereksmart merged 1 commit intomasterfrom
fix/wordads-undefined

Conversation

@dbspringer
Copy link
Copy Markdown
Member

@dbspringer dbspringer commented Dec 14, 2016 •
edited
Loading

screen shot 2016-12-14 at 10 40 45 am

This warning only manifests itself when running wp jetpack from the command line. This fixes the warning by ensuring HTTP_HOST is set.

@dbspringer dbspringer self-assigned this Dec 14, 2016
@dbspringer dbspringer added this to the 4.5 milestone Dec 14, 2016
@georgestephanis
Copy link
Copy Markdown
Contributor

georgestephanis commented Dec 14, 2016

This feels kinda awkward, I'd almost expect that WP-CLI would set that server var automatically.

@georgestephanis
Copy link
Copy Markdown
Contributor

georgestephanis commented Dec 14, 2016

Related: wp-cli/wp-cli#730 -- I guess their fix is just to document it and move on. Kinda kludgey, but whatevs.

🐑

@dbspringer
Copy link
Copy Markdown
Member Author

dbspringer commented Dec 14, 2016 •
edited
Loading

@georgestephanis the variable wouldn't be used for anything ads related from the cli anyway, this will just ensure it doesn't spit out any warnings.

@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Dec 16, 2016

Should we add this in class.jetpack.php or jetpack.php instead? There may be other places we would expect HTTP_HOST to be present and we can just blanket set it to avoid needing to duplicate this later.

Perhaps something like

if ( defined( 'WP_CLI' ) && WP_CLI ) {
$_SERVER['HTTP_HOST'] = parse_url( home_url(), PHP_URL_HOST );
}

@eliorivero
Copy link
Copy Markdown
Contributor

eliorivero commented Dec 16, 2016

Reproduced the issue, applied this and it effectively solves the issue, so LGTM 🐑

That said, maybe this can be wrapped in a condition checking defined( 'WP_CLI' ) && WP_CLI to avoid running it every time. Even more, maybe it can be added as Brandon mentions above, possibly in jetpack.php.

@eliorivero eliorivero 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 Dec 16, 2016
@dereksmart dereksmart merged commit 23dd0f3 into master Dec 16, 2016
@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Dec 16, 2016

branch-4.5 9e8a733

@dereksmart dereksmart deleted the fix/wordads-undefined branch December 16, 2016 18:39
jeherve added a commit that referenced this pull request Dec 21, 2016
@jeherve
Changelog: add #5874
12bb1cf
dereksmart pushed a commit that referenced this pull request Jan 17, 2017
@jeherve @dereksmart
Changelog: add #5866
d0a4697 
Changelog: add #5867

Changelog: add #5874

Changelog: add #5905

Changelog: add #5906

Changelog: add #5931

Changelog: add #5933

Changelog: add #5934
dereksmart pushed a commit that referenced this pull request Jan 17, 2017
@jeherve @dereksmart
Changelog: add #5866
10bc6c3 
Changelog: add #5867

Changelog: add #5874

Changelog: add #5905

Changelog: add #5906

Changelog: add #5931

Changelog: add #5933

Changelog: add #5934

Bring over 4.4.2 changelog from branch-4.4

@see 18012a3

Changelog: add #5976, #5978, #5983

Changelog: add #5917

Changelog: add #5832

Changelog: add 4.4.2 release post link.

CHangelog: add #5457

Changelog: add #5487

Changelog: add #5708

Changelog: add #5879

Changelog: add #5932

Changelog: add #5963

Changelog: add #5968

Changelog: add #5996

Changelog: add #5998

Changelog: add #5999

Changelog: add #6012

Changelog: add #6013

Changelog: add #6014

Changelog: add #6015

Changelog: add #6023

Changelog: add #6024

Changelog: add #6030

Changelog: add #5465

CHangelog: add #6063

Changelog: add #6025

Changelog: add #5974

Changelog: add #6059

Changelog: add #6046

Changelog: add #5418

Changelog: move things around and add missing information.

Changelog: add #5565

Changelog: add #6087

Changelog: add #6095

Readme: add @tyxla to the list of contributors.

Improved changelog for your readability and enjoyment

updated the release date

finalizing the changelog with a few more edits
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dereksmart dereksmart Awaiting requested review from dereksmart

Assignees

@dbspringer dbspringer

Labels

[Feature] Jetpack CLI [Feature] WordAds

Projects

None yet

Milestone

4.5

Development

Successfully merging this pull request may close these issues.

6 participants

@dbspringer @georgestephanis @kraftbj @eliorivero @dereksmart @jeherve