Skip to content

Fixes to make CI green#96

Merged
moufmouf merged 26 commits intothecodingmachine:masterfrom
lingoda:fix-ci
Jun 22, 2021
Merged

Fixes to make CI green#96
moufmouf merged 26 commits intothecodingmachine:masterfrom
lingoda:fix-ci

Conversation

@TamasSzigeti
Copy link
Copy Markdown
Contributor

@TamasSzigeti TamasSzigeti commented Jun 21, 2021

Several fixes:

None of this should be a breaking change

++ Some of this version bumps should go into the graphqlite package! – I can create a PR there later, but it doesn't hurt to have them here as well

@TamasSzigeti TamasSzigeti changed the title [WIP] GitHub CI Fixes [WIP] Fixes to make CI green Jun 22, 2021
@moufmouf
Copy link
Copy Markdown
Member

Woah, so cool! Thanks a lot for this PR!
It seems to me the CI is green. Can I merge this (or is it still WIP?)

@TamasSzigeti
Copy link
Copy Markdown
Contributor Author

Woah, so cool! Thanks a lot for this PR!
It seems to me the CI is green. Can I merge this (or is it still WIP?)

Perfect timing, I just finished (: Do review it please, though! Some comments/questions:

  • In the dependency cleanup I tried to keep things as loose as possible and only added constraints where it was already breaking. Maybe another time it would make sense to add even harder constraints e.g. symfony >=4.4 as anything below is already unsupported.
  • I haven't really looked into coveralls, do you still want to use it?
  • For now I kept travis.yml for reference but maybe it would make sense to just delete it?

@TamasSzigeti TamasSzigeti changed the title [WIP] Fixes to make CI green Fixes to make CI green Jun 22, 2021
@TamasSzigeti TamasSzigeti marked this pull request as ready for review June 22, 2021 07:19
@moufmouf
Copy link
Copy Markdown
Member

Perfect timing, I just finished (: Do review it please, though!

I did review it! Really good job 👍

Maybe another time it would make sense to add even harder constraints e.g. symfony >=4.4 as anything below is already unsupported.

Definitely, it would make sense for the next minor version!

I haven't really looked into coveralls, do you still want to use it?

That would be awesome. But we can save that for another PR.

For now I kept travis.yml for reference but maybe it would make sense to just delete it?

Let's keep it for now. There was a trick where I was uninstalling the "security bundle" and running tests again to test the behaviour of the graphqlite-bundle when there is no security bundle. We should probably reproduce that at some point.

@moufmouf moufmouf merged commit 5481ecb into thecodingmachine:master Jun 22, 2021
/**
* @Field
*/
public function getUserName(UserInterface $user): string
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are we expecting to rename this to getUserIdentifier in a future (breaking) 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