Skip to content
This repository was archived by the owner on Jan 31, 2023. It is now read-only.

pgo01org: class.php uses PHP 4 style constructors #51

Closed
cmb69 opened this issue Mar 21, 2019 · 5 comments
Closed

pgo01org: class.php uses PHP 4 style constructors #51

cmb69 opened this issue Mar 21, 2019 · 5 comments

Comments

@cmb69
Copy link
Contributor

cmb69 commented Mar 21, 2019

This may negatively affect the optimization (unless we're expecting such code to be still common). The problem has already been reported, but overall it appears that intel/php_pgo_training_scripts is unmaintained. Maybe we should fork?

@weltling
Copy link
Member

Thanks for bringing this up. I'm not sure if we can fork it under the Microsoft org, and if we can - whether it's worth the trouble. I'd suggest we consider one of the following

  • invent a patch and apply on init, or
  • bundle the source, compressed it should be fairly small

Frankly, i'd prefer the first option, as bundling is always something that should be avoided if not strictly necessary.

Thanks.

@cmb69
Copy link
Contributor Author

cmb69 commented Mar 27, 2019

I've filed intel/php_pgo_training_scripts#3. Maybe it'll be accepted; otherwise we could use it as patch.

@weltling
Copy link
Member

@cmb69 thanks for filing the PR upstream, but not sure to expect that one to be merged soon :) In might make sense integrating the patch at some point. Or actually not patch, but do a simple search/replace like here https://github.com/Microsoft/php-sdk-binary-tools/blob/master/pgo/cases/pgo01org/TrainingCaseHandler.php#L83. It won't fail, if the PR has been merged anyway.

Thanks.

@cmb69
Copy link
Contributor Author

cmb69 commented Mar 31, 2019

I've implemented a search and replace workaround as PR #53.

@weltling
Copy link
Member

PR merged.

Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants