cs3org / wopiserver

A vendor-neutral application gateway compatible with the WOPI specifications.
Apache License 2.0
52 stars 27 forks source link

Further lint the code #60

Closed glpatcern closed 2 years ago

glpatcern commented 2 years ago

Having integrated flake8 in the CI, a number of lint suggestions is available and could be implemented quite easily to further improve readability of the code base.

osamaahmed17 commented 2 years ago

Was fixing this issue, wanted to confirm whether you guys are using "Pylint" to lint the code? https://pypi.org/project/pylint/

pascalwengerter commented 2 years ago

You could also take a look at https://github.com/psf/black?

glpatcern commented 2 years ago

I know black, but that's a bit extreme, especially when it comes to line length (my personal opinion is that with nowadays screens, 120-130 chars per line are perfectly acceptable and make for more readable code).

Otherwise, the CI uses flake8 as mentioned and it's already a good reference.

shreyaansjain06 commented 2 years ago

I tried checking it with flake8

It mainly shows two types of error and this is one of them

E501 line too long (106 > 79 characters)

According to it, line should have almost 79 characters you are saying 120-130 chars per line

So am I missing something?

osamaahmed17 commented 2 years ago

That's great, it's merged. Horray!... We can make this issue resolved. I would have done it but I don't have rights.

glpatcern commented 2 years ago

Yes, the number of linting warnings was drastically dropped with the extra changes introduced in the PR.

Yet I will keep this issue open with a very low priority, for whenever we can at least partially address the remaining warnings (did not have time yesterday to also look for those). The current master build shows the following:

18    C901 '<function>' is too complex (14)
5     E124 closing bracket does not match visual indentation
5     E302 expected 2 blank lines, found 1
3     E305 expected 2 blank lines after class or function definition, found 1
29    E501 line too long (145 > 130 characters)
4     E741 ambiguous variable name 'l'
77
glpatcern commented 2 years ago

Closing this as with the latest developments we have brought the lint warnings to a handful.