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.

Sign up for GitHub

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

Jump to bottom

Code quality workflows #320

Closed
wants to merge 39 commits into from
Closed

Code quality workflows #320

wants to merge 39 commits into from

Conversation

kreneskyp
Copy link
Contributor

@kreneskyp kreneskyp commented Apr 6, 2023 •
edited
Loading

@Torantulino

This PR adds common code quality workflows. Goal is to provide a consistent sandbox environment for both local and github environments.

I based this off of #228 for the Dockerfile fixes it provided.

Changes

  • Github workflows that are optimized for build speed
    • Image is built and cached for re-use on subsequent runs
    • Cache key is based on hash of image inputs for re-use across similar branches
  • Adds makefile for local build and testing
  • Adds docker-compose file for adding other services to sandbox environment
  • Adds pytest, flake8, black, pyright, isort,

Testing:

TODO:

  • need to add mounts for sound device and test that it works on windows, linux, mac

For follow-up

  • I recommend completing this PR and then fixing errors for each test runner in a separate PR. Turn on branch protection for for each as they reach a passing state.

  • It would be worth optimizing the image to make it a bit smaller. It will be a bit friendlier to cache limits.

ksylvan and others added 30 commits April 4, 2023 23:49
@ksylvan
Fix docker build and add instructions in README.
05fbf6e
makefile and github workflows to:
22e91a4 
- build & push sandbox image
- run test runners, linters, and formatters
- shortcuts to run and shell
Merge remote-tracking branch 'ksylvan/master' into code_quality_workf…
8183ae7 
…lows
@kreneskyp
add docker-compose file, requirements-dev for testing tools, various …
e194e80 
…bugfixes
@kreneskyp
fix output reference
fcd0ed3
@kreneskyp
clean up action more
ba63e30
@kreneskyp
syntax fix
122c26e
@kreneskyp
fix escaping
71e7bdb
@kreneskyp
more cleanup
0a70ce1
@kreneskyp
reuse url
b10ee65
@kreneskyp
echo full url
ed96d67
@kreneskyp
fix build
7ff1f2f
@kreneskyp
fix build command
148abc2
@kreneskyp
fix build command
42780d1
@kreneskyp
reuse makefile tag/url generation, avoid pushing image for now
e110c7a
@kreneskyp
use cache to pass docker image
c2818e7
@kreneskyp
properly fetch url and tag
fb45f06
@kreneskyp
2nd attempt at loading image from cache
140f9ce
@kreneskyp
fix references to action
355314e
@kreneskyp
better path for local action
aa269d2
@kreneskyp
init test tasks properly, use image-url for cache key
7c4c9c2
@kreneskyp
fix docker run command
7973637
@kreneskyp
mechanism to bypass make build
a3a258e
@kreneskyp
update cache key
7abe0d1
@kreneskyp
default for image build bypass
bdbe5ab
@kreneskyp
remove bypass
766989e
@kreneskyp
add log message for cache key
3b62a20
@kreneskyp
skip build if cache exists
c021a43
@kreneskyp
add quotes
13f077e
@kreneskyp
fetch image url from make
641dfef
lcsmas
lcsmas previously approved these changes Apr 6, 2023
View reviewed changes
Copy link

@lcsmas lcsmas left a comment

Choose a reason for hiding this comment

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

LGTM

ksylvan
ksylvan approved these changes Apr 7, 2023
View reviewed changes
Copy link

@ksylvan ksylvan left a comment

Choose a reason for hiding this comment

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

LGTM

nponeccop
nponeccop suggested changes Apr 8, 2023
View reviewed changes
Copy link
Contributor

@nponeccop nponeccop left a comment

Choose a reason for hiding this comment

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

The scope is too big to sensibly merge. Split at least into 5 PRs:

  • gitignore corrections
  • updated docker build
  • github CI
  • readme typos, extra space and reformatting
  • readme changes and additions

@nponeccop
Copy link
Contributor

@kreneskyp Now it's out of date. Resolve the conflicts and split.

This was referenced Apr 10, 2023
Closed
Closed
@kreneskyp kreneskyp closed this Apr 14, 2023
waynehamadi added a commit that referenced this pull request Sep 5, 2023
@waynehamadi
Update pyproject.toml (#320)
3ce10df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nponeccop nponeccop nponeccop requested changes

@ksylvan ksylvan ksylvan approved these changes

@lcsmas lcsmas lcsmas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kreneskyp @nponeccop @ksylvan @lcsmas