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

Fixed CI #82

Merged
merged 3 commits into from
May 28, 2019
Merged

Fixed CI #82

merged 3 commits into from
May 28, 2019

Conversation

foxycode
Copy link
Contributor

It was little bit harder than I expected. I needed to add strict_types and return type hints. Not all checks could be fixed, I can't add return type hints where parent class don't have them for example. Hope it is ok this way.

@foxycode
Copy link
Contributor Author

@enumag Is it ok this way?

@enumag
Copy link
Member

enumag commented May 27, 2019

Fine by me.

@Vboss WDYT? Anything else before we can make a new release?

@foxycode
Copy link
Contributor Author

@enumag @Vboss I see Symfony 4.* tests failed, but they are in allowed failures. Was it ok before or not?

@foxycode foxycode mentioned this pull request May 27, 2019
@enumag
Copy link
Member

enumag commented May 27, 2019

@foxycode Other SF4 tests are passing. I'm guessing these are builds against dev symfony, which means 4.3 in this case. Since 4.3 will be released soon, I think we need to have this fixed.

@foxycode
Copy link
Contributor Author

@enumag I don't know why it is failing and I believe it's not related to my changes. You can probably merge this.

@Spamercz
Copy link
Member

Failing tests are related to changes in events in symfony 4.3-dev, more here Kdyby/Events#120 (comment)
There is some big BC break in arguments for events, I didnt looked into that closely but this should be fixed in another PR, because that BC break will probably persist to our Kdyby implementations too.

@Spamercz
Copy link
Member

Anyway PR is ok we can make release.

@enumag enumag merged commit 2ad30e8 into Kdyby:master May 28, 2019
@enumag
Copy link
Member

enumag commented May 28, 2019

Ah so the BC break will be solved in Kdyby/Events and doesn't directly affect Kdyby/Console? In that case we can make the new release.

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.

3 participants