pgo01org: class.php uses PHP 4 style constructors

microsoft / php-sdk-binary-tools

Tool kit for building PHP under Windows

BSD 2-Clause "Simplified" License

270 stars 79 forks source link

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

Closed cmb69 closed 5 years ago

cmb69 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

weltling commented 5 years ago

@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 commented 5 years ago

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

weltling commented 5 years ago

PR merged.

Thanks.