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

Set up PR validation #6

Closed
wants to merge 23 commits into from
Closed

Set up PR validation #6

wants to merge 23 commits into from

Conversation

molly-moen
Copy link
Collaborator

@molly-moen molly-moen commented Dec 6, 2024

Summary

Set up the following github actions on PRs:

  • Lint code
  • Run unit tests

The actions template I used is based on the recommended one for python packages.

I chose to use ruff for linting because it was popular, simple to setup and mentioned on the github actions documentation.

Right now there's only one unit test in the repo, for patch_requests, which we actually don't use right now, but it provided a convenient test case for this PR. I added a comment at the bottom of the script as to how we would run tests in additional folders. We will definitely have more tests in the upcoming neighborhood package.

Note: this PR does not have nearly as many lines as it seems. Most of those are auto-generated lines in the build files that can be ignored.

Links

Follow ups

I want to add a linting (and maybe also a run unit tests) pre-commit hook as well, but that work was separate enough that it will be done as a follow-up.

@molly-moen molly-moen changed the title Set up automation for the repo Set up PR validation Dec 6, 2024
@molly-moen molly-moen marked this pull request as ready for review December 6, 2024 22:25
@molly-moen molly-moen requested review from a team, snickell and fisher-alice December 6, 2024 22:25
Comment on lines +7 to +8
push:
branches: [ "main" ]

Choose a reason for hiding this comment

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

I don't think this is necessary. Nobody will be checking these post-merge actions. The primary goal here is to test PR's and get status feedback before merging.

strategy:
fail-fast: false
matrix:
python-version: ["3.12.2"]

Choose a reason for hiding this comment

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

lets go ahead and add 13 to the matrix!

Suggested change
python-version: ["3.12.2"]
python-version: ["3.12.2", "3.13.1"]

@molly-moen
Copy link
Collaborator Author

Closing in favor of this pr

@molly-moen molly-moen closed this Dec 11, 2024
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.

2 participants