Add Black Formatting #464

ethnh · 2022-02-02T01:24:44Z

Changes

Add Black Formatting workflows (Taken from https://github.com/cclauss/autoblack) 👍
- Format all code with the Black formatter ( See Black's Documentation or Black's PyPi page)
Update CONTRIBUTING.md

This change is entirely a style change -- There will be no impact on the code's performance from this
requirements.txt does not need to be updated -- this is not a requirement for running the code

Fixes

Inconsistent use of ' and "
Inconsistent use of tabs and spaces (if any exist)
(Tries to) Ensures code readability
Makes code style uniform
Update all code to follow PEP 8 (Read about Black's style here)

Checklist:

My code follows the style guidelines of this project and have read CONTRIBUTING.md (This adds style guidelines, I could not find any for this project)
I have performed a self-review of my own code
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works (I plan to add tests later)
I have checked my code and corrected any misspellings

rachmadaniHaryono · 2022-02-02T01:33:37Z

i don't know if this pr will be accepted, but maybe add pre commit black or use darker https://github.com/akaihola/darker

ethnh · 2022-02-02T01:59:21Z

Yes, pre-commit black would be the best option👍 Not every PR/push is made on the desktop, which is why I included the workflow. I'm 100% sure some commits would slip by unformatted if the workflow was removed, The Autoblack workflow has a really neat feature to automatically edit PR's or create PR's if any push does not pass Black's check I'd personally set it to do a daily run over the repo and run on every new PR, but that's just my opinion, so I left it as it was set in Autoblack's source

ethnh · 2022-02-02T02:05:19Z

If Theo wishes for the repo to have style guidelines, it'll be best to get them done sooner rather than later -- Merging already existing PRs after a large restyling like this could be a bit annoying, best to do it when there are few open PRs👍 If not, I'd get it removed from the pre-PR checklist 😂

rachmadaniHaryono · 2022-02-02T02:09:54Z

If Theo wishes for the repo to have style guidelines, it'll be best to get
them done sooner rather than later -- Merging already existing PRs after a
large restyling like this could be a bit annoying, best to do it when there
are few open PRs

i think only #424 change the main program

other than that other pr mainly for non python file

ThioJoe · 2022-02-02T02:23:35Z

I have literally no idea what this means

ethnh · 2022-02-02T02:29:44Z

It's about consistency across the codebase -- Some decent resources on the topic are: https://searchbusinessanalytics.techtarget.com/tip/Python-code-formatting-Tools-you-need-and-why-it-matters https://youtu.be/j1MbEYhYj_Y And https://www.python.org/dev/peps/pep-0008/ A great quote from the Python's PEP 8 is "One of Guido's key insights is that code is read much more often than it is written" Best to keep the codebase readable👍

rachmadaniHaryono · 2022-02-02T02:33:22Z

I have literally no idea what this means

simpler explanation

add github action to reformat pr
update contributing.md

is it too much @ThioJoe? if yes, maybe reduce the pr to essential and let thiojoe reformat the codebase

ethnh · 2022-02-02T02:46:21Z

The majority of the commit is changing single quotes to double quotes (and
likely tabs to spaces) to keep the code consistent, but at a quick glance
of the changes, there are definitely some big improvements

As a note to Thio: none of this changes any of the existing code, it only changes the way it looks

[...]
-            text =post['contentText']['runs'][0]['text'].strip().replace('\n','').replace('\r', '')
+            text = (
+                post["contentText"]["runs"][0]["text"]
+                .strip()
+                .replace("\n", "")
+                .replace("\r", "")
+            )
[...]

(Deleted original to fix markdown)

.github/workflows/black-formatting.yml

ethnh · 2022-02-02T03:12:07Z

i don't know if this pr will be accepted, but maybe add pre commit black or use darker https://github.com/akaihola/darker

@ThioJoe Also, if you are unaware of what a pre-commit is, I'd recommend checking out https://githooks.com/

The .git/hooks/pre-commit file tells git what to run before commiting files,
To add the black formatter to your pre-commit hooks, adding

python3 -m black

to the file should work👍

ethnh · 2022-02-02T03:54:01Z

Example run of black-push workflow (with the "auto fix" feature commented out)
https://github.com/EthanHindmarsh/YT-Spammer-Purge/runs/5031727716?check_suite_focus=true

dekoza · 2022-02-02T08:24:14Z

Adding pre-commit would also benefit this project.

KendallDoesCoding · 2022-02-02T10:39:30Z

Yes, pre-commit black would be the best option👍 Not every PR/push is made on the desktop, which is why I included the workflow. I'm 100% sure some commits would slip by unformatted if the workflow was removed, The Autoblack workflow has a really neat feature to automatically edit PR's or create PR's if any push does not pass Black's check I'd personally set it to do a daily run over the repo and run on every new PR, but that's just my opinion, so I left it as it was set in Autoblack's source

Yes, I use the desktop app sometimes, but I mostly use the web application to commit/make PR's

ethnh · 2022-02-02T14:54:36Z

let thiojoe reformat the codebase

Working on it right now 👍

UP929312 · 2022-02-02T15:18:10Z

I have literally no idea what this means

It's formatting which is objective so that everyone has to agree on following the same convention, that's what Black is, a forceful formatter that is pretty standard across python scripts.
A pre-commit hook or a workflow will mean that every time someone tries to add something to the codebase, it'll get automatically blackened (so automatically formatted) and is also pretty standard.

It has no effect on code functionality and is purely "cosmetic" in the sense that it's just so that everything follows formatting conventions.

This reverts commit 3ff6219.

This reverts commit 52ea96c.

This reverts commit 87402e0.

ethnh · 2022-02-02T15:31:58Z

Oops -- Workflow made that fixup! commit -- See https://github.com/EthanHindmarsh/YT-Spammer-Purge/actions/runs/1784682010
At least it shows it works 🤣

This reverts commit 6a40730.

ethnh · 2022-02-02T15:43:25Z

@ThioJoe The changes should be ready for review -- The workflow (example run) will automatically style & format the codebase after you merge this PR (if you do), so I recommend merging any other PRs you want before merging in this one.
Only four files modified:

Add Two workflows
- Run Black formatter on push
- Run Black formatter on pull requests
Update CONTRIBUTING.md
Add .pre-commit-config.yaml for those who use the pre-commit python package

ethnh · 2022-02-02T15:46:08Z

Changes reverted in Revert "Fix formatting issues with long dictionary names" can be re-added in a separate pull request should this be accepted 👍

ethnh

CONTRIBUTING.md should get a complete update, but I think adding a note about Black should be enough for this PR

rachmadaniHaryono · 2022-02-02T20:40:24Z

if you use pre commit, you may don't need to think about it

because every commit will be autoformatted

maybe some note on how to setup your fork on contributing.md, example below

set up your fork

fork the repo https://docs.github.com/en/get-started/quickstart/fork-a-repo
create virtualenv https://packaging.python.org/en/latest/guides/installing-using-pip-and-virtual-environments/#creating-a-virtual-environment
activate virtual env
install package pip3 install -r requirements.txt
install pre-commit pip3 install pre-commit
install pre-commit to git hook python3 -m pre-commit install

e: for poetry with pyproject

set up your fork

fork the repo https://docs.github.com/en/get-started/quickstart/fork-a-repo
install poetry https://python-poetry.org/docs/#installation
install with poetry poetry install
install pre-commit to git hook poetry run pre-commit install

ethnh · 2022-02-02T20:54:40Z

if you use pre commit, you may don't need to think about it

because every commit will be autoformatted

Not everyone commits from desktop:

Yes, I use the desktop app sometimes, but I mostly use the web application to commit/make PR's

maybe some note on how to setup your fork on contributing.md, example below

Yeah for sure 👍
See #489 -- His modified CONTRIBUTING.md is great

rachmadaniHaryono · 2022-02-02T21:51:28Z

Yeah for sure +1 See #489 --
His modified CONTRIBUTING.md is great

i have read it

unfortunately the pr change too much

but it is mostly correct

it describe how to setup the fork
how to check documentation
format/check/test the repo

i hope @dekoza can split the pr so better contributing.md can be used

dekoza · 2022-02-02T22:10:45Z

@rachmadaniHaryono I did what I could to minimize the changes. I used copier-poetry to create the basic directory structure and then moved most of the files into proper places. Then I refactored imports so that everything should work. Right now - since there are no tests - I'm trying to check if everything still works fine. If you ignore those several files added by copier-poetry, you'll see that the changes of imports are in fact small.

ethnh · 2022-02-04T21:28:11Z

Fetch upstream to ensure no merge conflicts ^^

rachmadaniHaryono · 2022-02-04T21:33:34Z

dekoza close 489, maybe you can copy pretty-format-yaml hook to make sure all yaml file have the same format

Taken from #489

ethnh · 2022-02-04T21:51:35Z

dekoza close 489, maybe you can copy pretty-format-yaml hook to make sure all yaml file have the same format

I updated .pre-commit-config.yaml to match #489
I'm not sure anything else from #489 is relevant to adding Black formatting -- the purpose of this PR

Maybe pyproject, but that seems more of a poetry thing

ethnh · 2022-02-04T21:53:31Z

Also, the failed check can be ignored, obviously, the current state of the repo would not pass a Black formatting check 😁

rachmadaniHaryono

also run pre commit autoupdate to make sure it use latest version

.pre-commit-config.yaml

rachmadaniHaryono · 2022-02-04T22:08:18Z

I'm not sure anything else from #489 is relevant to adding Black formatting -- the purpose of this PR

as far as i can tell only some hook pre commit

ethnh · 2022-02-04T22:49:58Z

also run pre commit autoupdate to make sure it use latest version

Doing this right now

As for the other issues you mentioned, I'll just comment out those parts of the yaml file for later 👍

ThioJoe · 2022-02-05T00:42:18Z

Tbh I don't really have any intention of adding this, I don't really understand the benefit

dekoza · 2022-02-05T21:10:38Z

Tbh I don't really have any intention of adding this, I don't really understand the benefit

Another brick in the wall...

Masamune3210 · 2022-02-06T01:29:44Z

Dude chill out, its not your repo. If you don't like a choice you don't have to deal with it lol

KendallDoesCoding · 2022-02-06T19:27:10Z

Dude chill out, its not your repo. If you don't like a choice you don't have to deal with it lol

Comment by Masamune3210

This comment was directed to someone else, not towards the PR or myself
@EthanHindmarsh

Who was this comment directed to @Masamune3210

ethnh · 2022-02-06T19:31:55Z

Dude chill out, its not your repo. If you don't like a choice you don't have to deal with it lol

Comment by Masamune3210

This comment was directed to someone else, not towards the PR or myself
@EthanHindmarsh

Who was this comment directed to @Masamune3210

Probably the comment above😂

KendallDoesCoding · 2022-02-06T19:33:25Z

Dude chill out, its not your repo. If you don't like a choice you don't have to deal with it lol

Comment by Masamune3210

This comment was directed to someone else, not towards the PR or myself
@EthanHindmarsh

Who was this comment directed to @Masamune3210

Probably the comment above😂

Ok lol, I think he was just joking though.

Add Black Formatting

87402e0

ethnh commented Feb 2, 2022

View reviewed changes

.github/workflows/black-formatting.yml Outdated Show resolved Hide resolved

ethnh added 2 commits February 1, 2022 21:32

Fix formatting issues with long dictionary names

52ea96c

Seperate push and pull into two different workflows

3ff6219

ethnh added 5 commits February 2, 2022 09:19

Revert "Seperate push and pull into two different workflows"

8d24221

This reverts commit 3ff6219.

Revert "Fix formatting issues with long dictionary names"

47f5e99

This reverts commit 52ea96c.

Revert "Add Black Formatting"

ad4e04e

This reverts commit 87402e0.

Simplify changes into four files

2dcfdd2

fixup! Format Python code with psf/black push

6a40730

Revert "fixup! Format Python code with psf/black push"

bdff28c

This reverts commit 6a40730.

ethnh commented Feb 2, 2022

View reviewed changes

Merge branch 'ThioJoe:main' into main

7de772d

Update .pre-commit-config.yaml

75003b9

Taken from #489

rachmadaniHaryono suggested changes Feb 4, 2022

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

.pre-commit-config.yaml Outdated Show resolved Hide resolved

.pre-commit-config.yaml Outdated Show resolved Hide resolved

.pre-commit-config.yaml Outdated Show resolved Hide resolved

Commented out parts until later & Updated versions.

6674a75

rachmadaniHaryono approved these changes Feb 4, 2022

View reviewed changes

ThioJoe closed this Feb 5, 2022

ethnh mentioned this pull request Feb 5, 2022

Reopen: Add Black formatting #573

Closed

8 tasks

Add Black Formatting #464

Add Black Formatting #464

Conversation

ethnh commented Feb 2, 2022 • edited

Changes

Fixes

Checklist:

rachmadaniHaryono commented Feb 2, 2022

ethnh commented Feb 2, 2022 via email • edited

ethnh commented Feb 2, 2022 via email • edited

rachmadaniHaryono commented Feb 2, 2022

ThioJoe commented Feb 2, 2022

ethnh commented Feb 2, 2022 via email • edited

rachmadaniHaryono commented Feb 2, 2022 • edited

ethnh commented Feb 2, 2022 • edited

ethnh commented Feb 2, 2022

ethnh commented Feb 2, 2022

dekoza commented Feb 2, 2022

KendallDoesCoding commented Feb 2, 2022 • edited

ethnh commented Feb 2, 2022

UP929312 commented Feb 2, 2022

ethnh commented Feb 2, 2022

ethnh commented Feb 2, 2022 • edited

ethnh commented Feb 2, 2022

ethnh left a comment

Choose a reason for hiding this comment

rachmadaniHaryono commented Feb 2, 2022 • edited

set up your fork

set up your fork

ethnh commented Feb 2, 2022 • edited

rachmadaniHaryono commented Feb 2, 2022

dekoza commented Feb 2, 2022

ethnh commented Feb 4, 2022

rachmadaniHaryono commented Feb 4, 2022

ethnh commented Feb 4, 2022

ethnh commented Feb 4, 2022

rachmadaniHaryono left a comment

Choose a reason for hiding this comment

rachmadaniHaryono commented Feb 4, 2022

ethnh commented Feb 4, 2022

ThioJoe commented Feb 5, 2022

dekoza commented Feb 5, 2022

Masamune3210 commented Feb 6, 2022

KendallDoesCoding commented Feb 6, 2022

ethnh commented Feb 6, 2022

KendallDoesCoding commented Feb 6, 2022

ethnh commented Feb 2, 2022 •

edited

ethnh commented Feb 2, 2022 via email •

edited

ethnh commented Feb 2, 2022 via email •

edited

ethnh commented Feb 2, 2022 via email •

edited

rachmadaniHaryono commented Feb 2, 2022 •

edited

ethnh commented Feb 2, 2022 •

edited

KendallDoesCoding commented Feb 2, 2022 •

edited

ethnh commented Feb 2, 2022 •

edited

rachmadaniHaryono commented Feb 2, 2022 •

edited

ethnh commented Feb 2, 2022 •

edited