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

Minor improvements #71

Merged
merged 7 commits into from Dec 13, 2018

Conversation

Projects
None yet
5 participants
@MathiasReker
Copy link
Contributor

MathiasReker commented Dec 5, 2018

Just a few minor improvements

@PierreRambaud PierreRambaud changed the base branch from master to dev Dec 5, 2018

@MathiasReker

This comment has been minimized.

Copy link
Contributor

MathiasReker commented Dec 5, 2018

@PierreRambaud Would it be better practice to use \n instead of \r\n ?

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Dec 5, 2018

The best practice is to use PHP_EOL const :)

MathiasReker added some commits Dec 5, 2018

Revert
Notice: Use of undefined constant _PS_CONFIG_DIR_ - assumed '_PS_CONFIG_DIR_' in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 30

Warning: include(_PS_CONFIG_DIR_/config.inc.php): failed to open stream: No such file or directory in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 30

Warning: include(): Failed opening '_PS_CONFIG_DIR_/config.inc.php' for inclusion (include_path='.;C:/laragon/etc/php/pear') in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 30

Notice: Use of undefined constant _PS_CONFIG_DIR_ - assumed '_PS_CONFIG_DIR_' in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 31

Warning: include(_PS_CONFIG_DIR_/../init.php): failed to open stream: No such file or directory in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 31

Warning: include(): Failed opening '_PS_CONFIG_DIR_/../init.php' for inclusion (include_path='.;C:/laragon/etc/php/pear') in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 31

Fatal error: Uncaught Error: Class 'Tools' not found in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php:34 Stack trace: #0 {main} thrown in C:\laragon\www\prestashop1.7.5\modules\gsitemap\gsitemap-cron.php on line 34
@MathiasReker

This comment has been minimized.

Copy link
Contributor

MathiasReker commented Dec 5, 2018

@PierreRambaud Consider to merge this PR as well :)

@MathiasReker

This comment has been minimized.

Copy link
Contributor

MathiasReker commented Dec 5, 2018

Does this PR need QA?

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Dec 5, 2018

Always need QA, always :D

@MathiasReker

This comment has been minimized.

Copy link
Contributor

MathiasReker commented Dec 5, 2018

Safety first 😄

@matks

matks approved these changes Dec 5, 2018

@MathiasReker MathiasReker referenced this pull request Dec 5, 2018

Closed

Update version #74

@MathiasReker

This comment has been minimized.

Copy link
Contributor

MathiasReker commented Dec 13, 2018

Ping @marionf

@marionf marionf added QA approved and removed Waiting for QA labels Dec 13, 2018

@marionf

This comment has been minimized.

Copy link

marionf commented Dec 13, 2018

Sorry @MathiasReker I was on the release tests last week for 1.7.5.0
It's ok for this one :)

@Quetzacoalt91 Quetzacoalt91 merged commit 7f6db05 into PrestaShop:dev Dec 13, 2018

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Dec 13, 2018

Thank you @MathiasReker

@MathiasReker MathiasReker deleted the MathiasReker:minor-improvements branch Dec 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment