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

build: update pre-commit pylint hook #190

Merged
merged 1 commit into from
Feb 27, 2024
Merged

Conversation

cesarcoatl
Copy link
Member

when pylint is installed isolated from pre-commit, e.g. with pipx,
it flags setup.py with E0401 since setuptools is not found
update tox config

when pylint is installed isolated from pre-commit, e.g. with pipx,
it flags setup.py with E0401 since setuptools is not found
update tox config
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Enhancement

PR Summary: This pull request introduces enhancements to the project's setup by updating the pre-commit configuration to specifically target Python files in the 'src' directory. Additionally, it updates the README.md to provide more accurate and external links for cloning the project, contributing, and the code of conduct. These changes aim to improve the developer experience by ensuring linting is more focused and by making documentation references more accessible and clearer.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
  • Unsupported files: the diff contains files that Sourcery does not currently support during reviews.

General suggestions:

  • Consider verifying the necessity and correctness of the query parameter in the updated cloning instructions URL to ensure it leads users to the intended information without confusion.
  • Review the specificity of the 'files' pattern in the .pre-commit-config.yaml to ensure it aligns with the project's directory structure and Python file organization. Adjusting the pattern might be necessary if Python files exist outside the 'src' directory that also require linting.
  • Ensure that external links to documents like CONTRIBUTING.md and CODE_OF_CONDUCT.md are stable and unlikely to change in the near future to prevent broken links.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@@ -89,11 +89,11 @@ To install incendium on your Gateway follow these steps:
1. If you're replacing a previous version, make sure to check Allow Overwrite
1. Click on **Import**

Alternatively you could follow the instructions for cloning the `project` branch directly into `$IGNITION_DIR/data/projects` found [here](https://github.com/ignition-incendium/incendium/tree/project#cloning-this-branch).
Copy link

Choose a reason for hiding this comment

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

nitpick (llm): The updated link for cloning instructions now points to a more specific location, which is helpful for users. However, ensure that the query parameter ?tab=readme-ov-file in the URL is necessary and correctly directs users to the intended section. If it's a typo or not required, it might confuse users or lead to a 404 page.

@cesarcoatl cesarcoatl merged commit e0ccace into main Feb 27, 2024
3 checks passed
@cesarcoatl cesarcoatl deleted the build/pre-commit-pylint-tox branch February 27, 2024 20:59
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.

1 participant