Skip to content

fix: improve linting and automation#172

Merged
mergify[bot] merged 2 commits intoinstructlab:mainfrom
RobotSail:improve-ci
Oct 1, 2024
Merged

fix: improve linting and automation#172
mergify[bot] merged 2 commits intoinstructlab:mainfrom
RobotSail:improve-ci

Conversation

@RobotSail
Copy link
Member

In the current version of the training repo, we have a few commands that help us lint
and check to see if there are any issues with types or incorrect imports. However;
there is no easy fix available when these issues are detected.

This commit introduces a few new functionalities:

  1. Makefile receives 5 new commands:
  • toml-lint, which lints the pyproject.toml file
  • toml-fmt, which automatically formats the pyproject.toml file
  • check-engine, which checks that the specified container engine ($CENGINE) is installed
  • tests, which runs tests against the source repo
  • fix, which fixes all fixable issues
  1. The tox.ini file receives a few additions:
    a. The testenv environment is defined with some preset values. Other targets are able to pull from this
    b. tox -e fix is added - automatically fixes all fixable problems
    c. tox -e tomllint is added - fixes all issues in the pyproject.toml file
    d. a [py] environment is added to specify the base python version

Aside from this, the resulting pyproject.toml file is added after formatting is applied.

Signed-off-by: Oleg S 97077423+RobotSail@users.noreply.github.com

@nathan-weinberg
Copy link
Member

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Aug 30, 2024

refresh

✅ Pull request refreshed

@mergify mergify bot added CI/CD Affects CI/CD configuration testing Relates to testing ci-failure labels Aug 30, 2024
Copy link
Member

@nathan-weinberg nathan-weinberg left a comment

Choose a reason for hiding this comment

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

Generally LGTM, not sure why a new lint error is getting picked up? Maybe needs a rebase?

@RobotSail
Copy link
Member Author

@mergify rebase

In the current version of the training repo, we have a few commands that help us lint
and check to see if there are any issues with types or incorrect imports. However;
there is no easy fix available when these issues are detected.

This commit introduces a few new functionalities:

1. Makefile receives 5 new commands:
 - toml-lint, which lints the pyproject.toml file
 - toml-fmt, which automatically formats the pyproject.toml file
 - check-engine, which checks that the specified container engine ($CENGINE) is installed
 - tests, which runs tests against the source repo
 - fix, which fixes all fixable issues

2. The tox.ini file receives a few additions:
  a. The testenv environment is defined with some preset values. Other targets are able to pull from this
  b. tox -e fix is added - automatically fixes all fixable problems
  c. tox -e tomllint is added - fixes all issues in the pyproject.toml file
  d. a [py] environment is added to specify the base python version

Aside from this, the resulting pyproject.toml file is added after formatting  is applied.

Signed-off-by: Oleg S <97077423+RobotSail@users.noreply.github.com>
@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2024

rebase

✅ Branch has been successfully rebased

Co-authored-by: Nathan Weinberg <31703736+nathan-weinberg@users.noreply.github.com>
Signed-off-by: Oleg <97077423+RobotSail@users.noreply.github.com>
@mergify mergify bot removed the ci-failure label Sep 26, 2024
@RobotSail
Copy link
Member Author

@nathan-weinberg Tests are passing now

@mergify mergify bot added the one-approval label Sep 27, 2024
Copy link
Contributor

@JamesKunstle JamesKunstle left a comment

Choose a reason for hiding this comment

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

looks like a good addition to me

@mergify mergify bot merged commit 63f128c into instructlab:main Oct 1, 2024
@mergify mergify bot removed the one-approval label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD Affects CI/CD configuration testing Relates to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants