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

DevKit updates for 1.x branch #170

Merged
merged 4 commits into from Feb 15, 2020
Merged

DevKit updates for 1.x branch #170

merged 4 commits into from Feb 15, 2020

Conversation

SonataCI
Copy link
Collaborator

No description provided.

@@ -3,3 +3,4 @@
.php_cs.cache
composer.lock
phpunit.xml
/.phpunit.result.cache
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add this to dev kit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole gitignore?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yes I think we could manage them mostly the same. Let's see afterwards if we have some special cases and add them via an {%if ...} like for other config files, WDYT? Open to create a PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do it

Copy link
Contributor

@greg0ire greg0ire Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we would list other config files in projects.yml or what?

@greg0ire
Copy link
Contributor

@OskarStark can you check PHPStan?

@OskarStark
Copy link
Member

@OskarStark can you check PHPStan?

I fixed the image itself, these are real errors reported by PHPStan. cc @core23 are you willing to fix them?

@greg0ire
Copy link
Contributor

these are real errors

To me it looks like composer deps are never installed and that's what causing the errors…

@OskarStark
Copy link
Member

The action is running:
Screenshot 2020-02-12 09 12 08

@greg0ire
Copy link
Contributor

And it complains about not being able to find classes from vendors

@greg0ire
Copy link
Contributor

@OskarStark I have no issue locally with phpstan so I think it really is an issue the workflow (maybe there should be more steps?)

@greg0ire
Copy link
Contributor

@OskarStark I think it is because the image uses composer install --no-dev: https://github.com/OskarStark/phpstan-ga/blob/8991b5f626c5d1f2f825916d22f6298269c2d9c9/entrypoint.sh#L3

This is incompatible with the concept of optional dependencies we sometimes use in Sonata.

@OskarStark
Copy link
Member

@silasjoisten can you please have a look?

@greg0ire
Copy link
Contributor

Let's merge as is for now, but it would be great to have some solution for this.

@greg0ire greg0ire merged commit 4a659ac into 1.x Feb 15, 2020
@greg0ire greg0ire deleted the 1.x-dev-kit branch February 15, 2020 12:57
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.

None yet

3 participants