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

chore: optimize GitHub CI workflow #220

Merged
merged 7 commits into from
Oct 24, 2023

Conversation

federicobond
Copy link
Member

@federicobond federicobond commented Oct 19, 2023

This PR

Optimizes our GH workflows by running linters like black/flake8 and CodeQL in a separate job with a single Python version, drastically reducing the feedback time for builds. Tests still run on a version matrix.

It also removes the virtualenv configuration which is not necessary in CI since the machines are ephemeral anyway, and uses the official Python setup actions instead of Docker to further reduce the number of layers involved in the execution of the build.

TODO:

  • once approved, repeat the same procedure for the python-sdk-contrib repo.

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #220 (61ebcbc) into main (291b4ae) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #220   +/-   ##
=======================================
  Coverage   93.63%   93.63%           
=======================================
  Files          16       16           
  Lines         440      440           
=======================================
  Hits          412      412           
  Misses         28       28           
Flag Coverage Δ
unittests 93.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@federicobond federicobond force-pushed the build-improvements branch 2 times, most recently from 15ebf19 to b841817 Compare October 20, 2023 18:37
@federicobond federicobond changed the title chore: test optimized GitHub workflow chore: optimize GitHub CI workflow Oct 20, 2023
@federicobond federicobond marked this pull request as ready for review October 20, 2023 18:44
@federicobond federicobond requested a review from beeme1mr October 20, 2023 18:45
@federicobond federicobond linked an issue Oct 20, 2023 that may be closed by this pull request
Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

The changes look good but do you need two separate files? I believe you should be able to update one of the files to support either trigger. It also may not be necessary to run at all on a merge to main since it was tested during the PR process.

@federicobond
Copy link
Member Author

Indeed, I just didn't want to change too much before validating the approach. Will update the PR.

@federicobond federicobond force-pushed the build-improvements branch 3 times, most recently from a7ead47 to 809d3d5 Compare October 22, 2023 01:18
.github/workflows/build.yml Outdated Show resolved Hide resolved
@federicobond
Copy link
Member Author

Not sure where those missing build status come from 🤔

federicobond and others added 6 commits October 22, 2023 18:52
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Federico Bond <federicobond@gmail.com>
@toddbaert
Copy link
Member

Not sure where those missing build status come from 🤔

I can't quite understand it either. I wonder if it's just an artifact of the changes and once it's merged it will work subsequently?

@toddbaert toddbaert self-requested a review October 24, 2023 18:34
@federicobond
Copy link
Member Author

@toddbaert the problem is that it does not let me merge without every status check passing. I will open a separate PR to see if that solves it.

@federicobond
Copy link
Member Author

@toddbaert I think it MIGHT have to do with some of these checks being marked as required and GH not preserving the mapping from the previous configuration. Do you have permission to disable them temporarily and then enable the new ones?

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
Copy link
Member

@federicobond we had branch protection enabled that look for the old job names.

@beeme1mr beeme1mr merged commit f74eda0 into open-feature:main Oct 24, 2023
11 checks passed
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.

Optimize our GitHub CI workflows
3 participants