Skip to content

13 formatter linter on pull#19

Merged
JustCallMeRay merged 15 commits intomainfrom
13-formatter-linter-on-pull
Sep 25, 2023
Merged

13 formatter linter on pull#19
JustCallMeRay merged 15 commits intomainfrom
13-formatter-linter-on-pull

Conversation

@fungss
Copy link
Copy Markdown
Collaborator

@fungss fungss commented Sep 18, 2023

Hi @JustCallMeRay

The below tasks are complete:
-[x] Have a formatter do a "dry run" (make no changes to the code but show up as a failed pipeline if a file requires formatting.
-[x] Have a linter do a dry run and fail the pipeline if any file requires changing

Please help check my work and let me know how it goes : ]
(Not sure if this is what you are looking for. I'm a little bit confused here. Seems like flake8 package does both the jobs, as linter and formatter, at the same time. It will dry run and report invalid formatting. Is something missing?)

Many thanks,

@fungss fungss linked an issue Sep 18, 2023 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@JustCallMeRay JustCallMeRay left a comment

Choose a reason for hiding this comment

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

Not bad, mostly small changes

Comment thread .github/workflows/unit-test.yaml Outdated
Comment thread funcs/__init__.py Outdated
Comment thread funcs/func.py Outdated
Comment thread funcs/func.py Outdated
Comment thread funcs/func_misformatted.txt Outdated
Comment thread tests/__init__.py Outdated
Comment thread tests/test_func.py Outdated
Comment thread tests/test_func.py Outdated
Comment thread requirements.txt Outdated
Copy link
Copy Markdown
Contributor

@JustCallMeRay JustCallMeRay left a comment

Choose a reason for hiding this comment

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

small things again!,Looking good though!

Comment thread .github/workflows/linting.yaml Outdated
Comment thread setup.cfg Outdated
@@ -0,0 +1,4 @@
[flake8]
max-line-length = 120
exclude =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also exclude .git, pycache, .idea, .vs and .vscode as this will make it a little quicker. We should really add them all to a .gitignore but we'll do that in another ticket.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

/venv/ is excluded in .gitignore by default and it won't be tested by flake8 when in a container. The line has been removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

People will also want to run the formatter on their machines (to actually make changes), if we add all the excludes it will run a little quicker for them and hopefully not ruin random files (like those in git)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok. Excluded unwanted directory in setup.cfg according to https://michaelcurrin.github.io/dev-cheatsheets/cheatsheets/python/linting/flake8.html

@JustCallMeRay JustCallMeRay self-requested a review September 25, 2023 10:24
@JustCallMeRay JustCallMeRay merged commit 7f41831 into main Sep 25, 2023
@fungss fungss deleted the 13-formatter-linter-on-pull branch September 29, 2023 17:18
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.

Run a formatter on pull request.

2 participants