search

destream-py / destream

A tool & Python 3 library to decompress anything
GNU General Public License v2.0
12 stars 1 forks source link

Add an .editorconfig #18

Open cecton opened 3 years ago

cecton commented 3 years ago

I don't know if it is possible but it would be nice to have a GitHub Actions enforcing it on the PR so it will give less work to the maintainers.

eumiro commented 3 years ago

Never used .editorconfig, but I have already adopted black and pre-commit hooks elsewhere that check at each PR for formatting and syntax. Wouldn't it be a more universal solution?

jruere commented 3 years ago

Black appears to re-format code directly and .editorconfig appears to just check and integrates with a bunch of editors to report, right?

Black seems very promising but I've never used that before. I'll try to implement it in some personal projects to understand how it works.

eumiro commented 3 years ago

black has almost no parameters, but there's one parameter that should be discussed. Its default line length is 88 chars. Some prefer to change it to some other number. I usually go with -l 79.

black does not have to reformat the code automatically. It has also a --diff parameter, so it just checks and complains if the file is not formatted correctly.

It's not that we absolutely have to use black. Try it out and let's discuss what works for us. However I'd prefer not to touch my text editor as I am using it for other projects as well.

It would also allow us to get rid of backslashes at the end of long lines, which is one thing I prefer not to have in my code. I'm open to (almost) any line length, to any (pep8ish) code format, but please no backslashes.

Have a look at pre-commit as well. There are many tools to keep the code in some rigid form. You can see it in action at https://github.com/eumiro/pyed, which is my older project with relatively modern infrastructure.

cecton commented 3 years ago

I think it is fine to make a commit to run black on the whole code so it is uniformized.

Back in my days there wasn't much option so it was formatted by hand. I don't mind you ruining my hand-crafted work at all, don't worry!! :grin:

cecton commented 3 years ago

I should have say "artisanal formatting"

cecton commented 3 years ago

pre-commit is fine but I recommend that you add a CI check! (I hate pre-commit, I never use that kind of stuff and I'm not an exception, a lot of people feel that way).

(That doesn't mean you shouldn't use the pre-commit hook. I understand it's important for some people)

cecton commented 3 years ago

I mean, you need both

eumiro commented 3 years ago

There's a CI check for pre-commit, relatively straightforward: https://github.com/eumiro/pyed/blob/main/.github/workflows/pre-commit.yml

And there's the configuration for pre-commit, that has quite many points we could/should discuss if we want to include them: https://github.com/eumiro/pyed/blob/main/.pre-commit-config.yaml

So it is not necessary to have pre-commit on one's client. PR's CI will tell you what's wrong. And since most PRs are atomic (ehm, ehm, ehm), there should be infrequent problems.

As @cecton says, there must be one commit, that changes the whole artisanal formatting and breaks all git blame entries. Then all future commits will be much easier to read, because black prepared the code accordingly.

And such a commit can appear in the future again. When the Python versions migration brings new syntax elements (such as Python 2→3 migration, pathlib.Path, f-strings, await/async keywords, etc.) or when there's a new version of black with a new opinion on how some syntax elements should look like. But that's because software is built like our cities: on ruins.

jruere commented 3 years ago

pre-commit is fine but I recommend that you add a CI check! (I hate pre-commit, I never use that kind of stuff and I'm not an exception, a lot of people feel that way).

(That doesn't mean you shouldn't use the pre-commit hook. I understand it's important for some people)

I've never used pre-commit. Could you elaborate on why you hate it?

cecton commented 3 years ago

I prefer to commit often and without spending time on commit messages. So usually my commit history is full of "WIP".

It looks dirty at first but at the end I just squash merge the whole PR and write a beautiful message. So on the main branch it always look perfect.

Pushing often allows me to see the result of the CI sooner and get review feedbacks faster to.

But in OSS unfortunately you often wait very long before getting an answer...

jruere commented 3 years ago

Thank you.