Skip to content
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

git hub actions #29

Open
wants to merge 1 commit into
base: v1-develop
Choose a base branch
from

Conversation

DominicWatts
Copy link
Contributor

docker container based github actions to test code instead of makefile which for example relies on components being available locally. Plus get added automatic code scan on any PRs

key is in action so publicly visible but no reason couldn't utilise github secrets for key

this is more a demo of what can be done

Results look like this:

https://github.com/DominicWatts/magento2-module-url-data-integrity-checker/actions

You don't have to use container approach e.g. https://github.com/DominicWatts/Menu/blob/master/.github/workflows/standards.yml

However the reason I use container based approach is it's easier to switch php version and also for phpstan in particular not to throw false positives if components from M2 required modules are used : php_intl - e.g. IntlDateFormatter not found etc.

https://github.com/DominicWatts/PHPStan

But certainly container based unit tests are difficult

@DominicWatts
Copy link
Contributor Author

plus you can give your project a nice badge

🥇

@hostep
Copy link
Member

hostep commented Sep 26, 2021

Thanks for the effort!

I really hope that those repo.magento.com credentials are not valid, otherwise I would strongly suggest you delete them and create new ones.

And I don't really agree with the changes in the phpstan.neon file. I want to be very explicit about which error to ignore in which file, otherwise it might mean that we hide warnings that shouldn't be hidden.

But I'll try to check it the PR out in more detail when I find some more time.

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

Successfully merging this pull request may close these issues.

2 participants