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

[Workflow] Automatically Lint and Format PRs #116

Merged
merged 11 commits into from
Apr 3, 2024
Merged

[Workflow] Automatically Lint and Format PRs #116

merged 11 commits into from
Apr 3, 2024

Conversation

benjaminye
Copy link
Contributor

Addresses #103 and partially #106

New Github Action

  • Added Github action to Lint and Format using ruff
  • Added configs for ruff
  • Automatically commit code changes onto PR

Lint and Format

  • Linted and Formatted codebase (minus scripts within llama2 and mistral)

Request for Comment

@truskovskiyk

  • Right now it's set up to automatically commit changes lint and formatting fixes directly onto the PR.
  • Is this alright?
  • This requires read/write access for github workflow runners under repo settings

@benjaminye benjaminye added the enhancement New feature or request label Apr 2, 2024
@benjaminye benjaminye self-assigned this Apr 2, 2024
truskovskiyk
truskovskiyk approved these changes Apr 3, 2024
View reviewed changes
Copy link
Contributor

@truskovskiyk truskovskiyk left a comment

Choose a reason for hiding this comment

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

thanks, much nicer!

README.md Outdated
Comment on lines 259 to 264
### Checklist Before Pull Request (Optional)

1. Use `ruff check` to check for lint errors
2. Use `ruff format` to apply formatting

NOTE: Ruff linting and formatting are done automatically when PR is raised using Git Action (and changes will be automatically applied via another commit). It is, however, a good practice to check and fix lint errors, as well as apply formatting before PR.
Copy link
Contributor

Choose a reason for hiding this comment

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

might be nice to add this into Makefile after

@truskovskiyk
Copy link
Contributor

@benjaminye

Right now it's set up to automatically commit changes lint and formatting fixes directly onto the PR.
Is this alright?

No, give access CI to write code into main branch is not best idea, let's just keep check-in CI - if code does not follow style - PR fails. but this exact fix - need to be done by author of PR

@benjaminye
Copy link
Contributor Author

@benjaminye

Right now it's set up to automatically commit changes lint and formatting fixes directly onto the PR.
Is this alright?

No, give access CI to write code into main branch is not best idea, let's just keep check-in CI - if code does not follow style - PR fails. but this exact fix - need to be done by author of PR

@truskovskiyk
Amended desired changes, as required. See 36688e0

@benjaminye benjaminye merged commit 85f7f7b into main Apr 3, 2024
2 checks passed
@benjaminye benjaminye linked an issue Apr 3, 2024 that may be closed by this pull request
[Workflow] Automatically run black, flake8, isort via Github Action #103
Closed
@benjaminye benjaminye mentioned this pull request Apr 3, 2024
Closed
@benjaminye benjaminye deleted the feature/format-lint-action branch April 4, 2024 02:41
@benjaminye benjaminye linked an issue Apr 5, 2024 that may be closed by this pull request
Add GitHub Actions CI for checking style, running tests, publishing Docker images, PyPI packages, and documentation. #106
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@truskovskiyk truskovskiyk truskovskiyk approved these changes

@RohitSaha RohitSaha Awaiting requested review from RohitSaha

Assignees

@benjaminye benjaminye

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
2 participants
@benjaminye @truskovskiyk