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

Added travis + tests + cs fixes #24

Merged
merged 5 commits into from Oct 18, 2018

Conversation

Projects
None yet
4 participants
@mickaelandrieu
Contributor

mickaelandrieu commented Oct 16, 2018

This contribution is about adding premises of a test suite and coding styles.

Also, a configuration for Travis.

@@ -19,7 +19,7 @@
* needs please refer to http://www.prestashop.com for more information.
*
* @author PrestaShop SA <contact@prestashop.com>
* @copyright 2007-2016 PrestaShop SA
* @copyright 2007-2016 PrestaShop SA

This comment has been minimized.

@PierreRambaud

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Oct 17, 2018

Contributor

if I update this one I need to update all the others, I'd like to keep this pull request with one scope only

Show resolved Hide resolved .travis.yml Outdated
"scripts": {
"cs-fix": "@php ./vendor/bin/php-cs-fixer fix .",
"cs-fix-test": "@php ./vendor/bin/php-cs-fixer fix --dry-run --stop-on-violation --show-progress=dot .",
"test": "SYMFONY_PHPUNIT_VERSION=5.7 php ./vendor/bin/simple-phpunit"

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Oct 17, 2018

Contributor

interesting point here: passing env variables here make the special @php unavailable in Composer scripts. I don't know if it's intended or not.

@matks

matks approved these changes Oct 18, 2018

@matks

This comment has been minimized.

Contributor

matks commented Oct 18, 2018

@mickaelandrieu Wen being shipped to Addons as a downloadable PS module, do you know what process is performed ? I think there is at least 1 composer install.

  • is the composer install performed for DEV or PROD
  • shall we remove the test files (phpunit.xml, .travis.yml) and folders (tests and tools) as they are not useful for the module to be executed/installed ?
@matks

This comment has been minimized.

Contributor

matks commented Oct 18, 2018

⚠️ this PR targets master instead of dev, is that intended ?

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Oct 18, 2018

is the composer install performed for DEV or PROD?

I don't think composer is called at all, that's why I don't use it to autoload module classes

shall we remove the test files (phpunit.xml, .travis.yml) and folders (tests and tools) as they are not useful for the module to be executed/installed ?

We could, but how? I'm thinking about using deploy feature of travis ci to perform these tasks, but this wont be addressed here.

this PR targets master instead of dev, is that intended ?

I don't know because I can't figure out why we have both dev and master branches on every module :/ ping @Quetzacoalt91

@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Oct 18, 2018

  • Composer is run with --no-dev parameter when deployed to the marketplace. This allow the zip to not provide useless stuff for the merchants.

  • dev contains the current version in progress. Once we are ready to publish a new version, we merge dev into master to trigger a new release on the marketplace the next night.

@mickaelandrieu mickaelandrieu changed the base branch from master to dev Oct 18, 2018

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Oct 18, 2018

I've switched to dev and as I have only dev dependencies, there won't be any impact for the marketplace.

Thanks for the answers @Quetzacoalt91 !

@matks matks merged commit 6756078 into PrestaShop:dev Oct 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@matks

This comment has been minimized.

Contributor

matks commented Oct 18, 2018

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