Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adopt pre-commit-ci for linting and typecheck #364

Merged
merged 7 commits into from
May 16, 2023
Merged

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented May 8, 2023

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

@achimnol achimnol self-assigned this May 8, 2023
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Could you integrate running pre-commit in the normal CI too?

@webknjaz webknjaz closed this May 8, 2023
@webknjaz webknjaz reopened this May 8, 2023
@webknjaz
Copy link
Member

webknjaz commented May 8, 2023

Reopening the PR was needed to trigger the pre-commit.ci app check. Now that it's been reported, I've made it required in the branch protection.

.pre-commit-config.yaml Outdated Show resolved Hide resolved
.yamllint Show resolved Hide resolved
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

I suppose this is good to go now.

@webknjaz webknjaz closed this May 10, 2023
@webknjaz webknjaz reopened this May 10, 2023
@webknjaz
Copy link
Member

@achimnol I've closed+reopened the PR to test the updated RTD integartion per #352 (comment). All's in order now.

@achimnol achimnol enabled auto-merge (squash) May 16, 2023 06:47
@achimnol achimnol merged commit 9ec9910 into main May 16, 2023
@achimnol achimnol deleted the ci/adopt-pre-commit-ci branch May 16, 2023 06:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants