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

added pep8 check function #158

Conversation

Grashalmbeisser
Copy link
Collaborator

No description provided.

@Grashalmbeisser Grashalmbeisser changed the title added pep8 check added pep8 check function May 24, 2024
src/viur_cli/local.py Outdated Show resolved Hide resolved
Co-authored-by: Sven Eberth <mail@sveneberth.de>
@sveneberth sveneberth changed the title added pep8 check function feat: Add pep8 check function Aug 2, 2024
@sveneberth sveneberth changed the title feat: Add pep8 check function added pep8 check function feat: Add pep8 check function Aug 2, 2024
@sveneberth sveneberth changed the title added pep8 check function feat: Add pep8 check function added pep8 check function Aug 2, 2024
Copy link
Member

@phorward phorward left a comment

Choose a reason for hiding this comment

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

Hi!

The purpose of this function is not clear to me. We currently try to distribute a tox.ini file with every project, which also integrates with IDE linters, so that files are checked for PEP8 during editing already.

IMHO, viur-cli should focus on project setup #165, etc. to maintain project files automatically.

utils.echo_info("\U00002714 No vulnerabilities found.")

def checkpep8(path):
ignore_list = ["E121", "E123", "E126", "E133", "E226", "E241", "E242", "E704", "W503", "W504", "W505"]
Copy link
Member

Choose a reason for hiding this comment

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

Can this definition be used from the tox.ini file provided with the project?

Copy link
Member

@sveneberth sveneberth left a comment

Choose a reason for hiding this comment

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

The argumentation for or against this is similar as in #160.

IMO the viur-cli doesn't need to implement a CLI command for another software which has already an CLI.


I have an alternative suggestion:

Add a corresponding default command to the viur-base's Pipfile, like https://github.com/viur-framework/viur-core/blob/dd0431bc14c2a2b897f2f54de092cac45b0d5e71/Pipfile#L21. At the end is the project logic.
And the CLI then executes defined commands in the project.json as pre-action hooks.

For example:

{
    "//": "[...]",
    "default": {
        "application_name": "myproject-with-hooks",
        "actions": {
            "pre-deploy": [{
                "command": "pipenv run linter",
                "kind": "exec",
            }, {
                "kind": "viur-cli-build",
            }],
            "pre-devserver": [{
                "command": "pipenv run dependency_check",
                "kind": "exec",
            }, {
                "command": "npm run watch",
                "kind": "exec-background",
            }],
        },
    },
    "//": "[...]",
}

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants