gtfspy icon indicating copy to clipboard operation
gtfspy copied to clipboard

Add pre commit configuration (original, reworking)

Open evelyn9191 opened this issue 5 years ago • 2 comments

The functionality of this library is awesome. Let's make the code awesome, too.

It is difficult to do changes in the current code. With new and new contributors, it could become even more difficult. I believe that following some basic Python developing good practices can help us to make the code more clean.

I fixed the formatting of the current code as much as I could and added linting checks to make us write cleaner code.

Besides the commit changing the formatting as suggested by Black, other commits should be reviewed with regards to whether or not the change will influence the functionality of the library.

I also needed to change a bit the installation of Cython. Btw, before, there were errors thrown during the libraries installations that the gtfspy couldn't be built/couldn't get its wheels or similar. Yet, the build continued happily and the tests passed. If you know why it didn't fail although an error was raised, it would be great if you could share the reason.

evelyn9191 avatar Feb 01 '20 18:02 evelyn9191

Historically I haven't been a big fan of automatic code formatting, because there are useful context cluses for me as a developer in the formatting - especially vertical space and patterns in comments. But, if someone does good work, we should use it.

I guess this might conflict with some of the other things that will come soon. Is it easy to split this into different PRs that do each different component? I see a lot of commits here, if each is manual work, we can try to do it as-is. I just need some time to review it all.

rkdarst avatar Mar 02 '20 21:03 rkdarst

Some are manual work, some not. I agree it is a large PR and it should be split. I have taken out the first part where the basic pre-commit config and Cython stuff is solved (including your comment above) and Black did the automatic formatting. See here.

I would keep this PR open, though, and depending on how the splitting would go due to the amount of the manual work and the new changes that may be merged in the meantime to master, decide whether to rebase and adjust this one or create new ones entirely.

evelyn9191 avatar Mar 07 '20 07:03 evelyn9191