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

Build with DBAL 3 #8874

Merged
merged 1 commit into from
Aug 3, 2021
Merged

Build with DBAL 3 #8874

merged 1 commit into from
Aug 3, 2021

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Aug 2, 2021

Please consider reviewing and merging #8872 first.

This is a work in progress, the plan as of now is to have 3 additional build jobs:

  • 1 for phpstan
  • 1 for Psalm
  • 1 for Phpunit with SQLite

When those 3 are green, the next steps should be to run the PHPUnit jobs on more PHP versions (it only runs on 8.0 currently), and then to add more jobs depending on that job.

For static analysis, I used a variable, but for continuous integration, I don't think it's possible since it would make dependent jobs (about postgres, mysql, etc.) not run.

While I'm confident we will be able to fix the phpunit job, it might be more challenging to fix the Phpstan job without making the Phpstan job with DBAL 2 fail because it does not have the exact same baseline. We should not hesitate to remove it if that turns out to be the case IMO, but until then, it's good to have that information in the pipeline and ask the community for help in getting that DBAL 3 support 🙂

@greg0ire greg0ire force-pushed the build-with-dbal-3 branch 3 times, most recently from 14fa43f to 6d59e23 Compare August 2, 2021 20:40
@greg0ire
Copy link
Member Author

greg0ire commented Aug 2, 2021

Maybe it would be better to use duplication for static analysis too 🤔
That way, we can customize the job title with something like Experimental, please disregard failures

Also, it looks like having a failure in a matrix causes cancellation of other jobs of that matrix, so it would be one more reason not to use a variable.

@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

Switching to fail-fast: false for now.

@greg0ire greg0ire force-pushed the build-with-dbal-3 branch 3 times, most recently from d6d39ac to 08aa97d Compare August 3, 2021 06:20
@greg0ire greg0ire marked this pull request as ready for review August 3, 2021 06:30
@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

Cc @derrabus

@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

I suggest we mark the regular static analysis jobs as required after merging this (I'm not sure why they were not in the first place)

ostrolucky
ostrolucky previously approved these changes Aug 3, 2021
@derrabus
Copy link
Member

derrabus commented Aug 3, 2021

This pipeline looks good to me. The only concern I have is that to a external contributor, it might not be obvious which CI jobs are expected to fail at the moment.

derrabus
derrabus previously approved these changes Aug 3, 2021
@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

@derrabus let me try to make it more obvious by using strings instead of booleans

derrabus
derrabus previously approved these changes Aug 3, 2021
morozov
morozov previously approved these changes Aug 3, 2021
@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

I'm adding continue-on-error: true because it probably has an influence on the global status.

@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

It does not seem sufficient, but I think I found an ugly solution: actions/runner#2347

Making these jobs green will not result in a comprehensive result of the
features, but it is a good start, and having them should give a good
overview of what is left to do.
@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

Ok so now the situation is: we are pretending experimental jobs are green when they really aren't. That's great because the project still looks healthy (the global status is green), contributors aren't spooked, we can check the status of each job, and we can mark them as non-experimental as soon as they are green. WDYT?

@morozov
Copy link
Member

morozov commented Aug 3, 2021

To me, sounds exactly what the project needs. Until the build is green, each patch will be merged at the reviewer's discretion.

@morozov
Copy link
Member

morozov commented Aug 3, 2021

It does not seem sufficient, but I think I found an ugly solution: actions/runner#2347 (comment)

I'm curious if it could be implemented in the DBAL to re-enable PHP 8.1 builds.

@greg0ire
Copy link
Member Author

greg0ire commented Aug 3, 2021

I'm curious if it could be implemented in the DBAL to re-enable PHP 8.1 builds.

I think it could 👍

@greg0ire greg0ire merged commit acdbbda into doctrine:2.10.x Aug 3, 2021
@greg0ire greg0ire deleted the build-with-dbal-3 branch August 3, 2021 21:52
@greg0ire greg0ire added the CI label Aug 4, 2021
@greg0ire greg0ire added this to the 2.10.0 milestone Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants