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

Feature/pre hook #14

Merged
merged 3 commits into from
Oct 13, 2023
Merged

Feature/pre hook #14

merged 3 commits into from
Oct 13, 2023

Conversation

epplepascal
Copy link
Contributor

@epplepascal epplepascal commented Oct 13, 2023 •
edited
Loading

It is now possible to add visiumlint as a pre-commit hook.

The following changes were made:

  • Added a --hook argument. This boolean argument, which is set to False by default, will not run Pylint when set to true. This was done since the the pre-commit runs visiumlint from an isolated virtualenv. Many of pylint's checks perform dynamic analysis which will fail there (such as imports). The hook still runs Pylint but as a local hook. This is specified in the .pre-commit-config.yaml file.
  • Added a .pre-commit-hooks.yaml file containing the pre-commit that executes visiumlint without Pylint
  • Modified the ReadMe to explain how to use the hook.

Pascal Epple added 2 commits October 13, 2023 14:01
Gramet
Gramet approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@Gramet Gramet left a comment

Choose a reason for hiding this comment

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

Minor changes in the README, otherwise looks good!

README.md Outdated

You can automate visiumlint when commiting changes with a [git hook](https://githooks.com/) and the [pre-commit](https://pre-commit.com/) library.

- Make sure to have installed pre-commit, or else run `brew install pre-commit`
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather recommend pip install pre-commit

README.md Outdated
language: python
types: [python]
require_serial: true

Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove this (l.36-44), might as well not run pylint at all in the hook

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@epplepascal epplepascal merged commit 9179d9e into main Oct 13, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gramet Gramet Gramet approved these changes

@ruaultadrien ruaultadrien Awaiting requested review from ruaultadrien

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@epplepascal @Gramet