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

Add lint aspect for Ruff #16

Merged
merged 11 commits into from
Oct 25, 2023
Merged

Add lint aspect for Ruff #16

merged 11 commits into from
Oct 25, 2023

Conversation

tokongs
Copy link
Contributor

@tokongs tokongs commented Oct 16, 2023

This project looks interesting!

I thought I'd try it out by creating a lint aspect for Ruff. And with that done, I might as well submit a PR for it too.

This adds support for the Ruff Python linter. I was unsure how you'd like to structure the binary download in the example.


Type of change

  • New feature or functionality (change which adds functionality)

For changes visible to end-users

  • Relevant documentation has been updated

Test plan

  • Covered by existing test cases

@CLAassistant
Copy link

CLAassistant commented Oct 16, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

Awesome thanks! Could you rebase on main?

lint/ruff.bzl Outdated Show resolved Hide resolved
example/WORKSPACE.bazel Outdated Show resolved Hide resolved
docs/BUILD.bazel Show resolved Hide resolved
@tokongs tokongs force-pushed the tobias/ruff branch 2 times, most recently from 296798e to 81fce5d Compare October 17, 2023 08:29
Copy link
Member

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

Thanks!

example/BUILD.bazel Outdated Show resolved Hide resolved
@alexeagle
Copy link
Member

Oh, can you sign our CLA also? We need legal permission to use your work.

@tokongs
Copy link
Contributor Author

tokongs commented Oct 19, 2023

Thanks for the quick feedback! I'm just awaiting a go-ahead from legal at my workplace before signing the CLA.

@tokongs
Copy link
Contributor Author

tokongs commented Oct 19, 2023

@alexeagle I'm sadly not quite sure I'll be able to sign the CLA with the current License the project has, as we will not be able to use it.

@tokongs
Copy link
Contributor Author

tokongs commented Oct 19, 2023

I'll close this for now, but I can re-open it in the future if the license changes. Thanks for the rapid feedback and sorry for not seeing the license earlier!

@tokongs tokongs closed this Oct 19, 2023
@alexeagle
Copy link
Member

This repo is now licensed Apache 2.0, largely because I think we'll only get critical mass of linter integrations with contributions like this one :)

@alexeagle alexeagle reopened this Oct 19, 2023
@tokongs
Copy link
Contributor Author

tokongs commented Oct 24, 2023

Nice! I've signed the CLA now :)

@alexeagle alexeagle merged commit 964d0e2 into aspect-build:main Oct 25, 2023
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.

3 participants