Adopt style guide + checks

[steven-esser]

@pombredanne @JonoYang @AyanSinhaMahapatra @sbs2001

We need to make a decision as to what our python style approach should be. We have in the past a mix of PEP8 and some using black. I am not opposed to any method, we just need to decide on something.

Thoughts?

[JonoYang]

@MaJuRG I think it would be best to run black before committing code or (if possible) have some way to run black automatically on code that we push to a repo.

[sbs2001]

@JonoYang

For

run black before committing code

That can be handled by using a git pre-commit hook alright. We can add that too in the skeleton .

have some way to run black automatically on code that we push to a repo.

This can be done via github action or even as a part of travis.

Having said that I don't think it would be a good idea to adopt black 100%. Rather incremental adoption seems a better/safer approach, especially in the context of huge codebase like scancode. For this we could run black only on the files which are changed.

Do note that black has default line limit of 88 characters, which might need to be overridden with some other limit.

Unrelated :
@MaJuRG as this repo is a template for the org's projects, why not make it a template repo ? https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/creating-a-template-repository .

[AyanSinhaMahapatra]

@sbs2001 that makes a lot of sense.

Using black in a pre-commit hook and addition to Travis (like with some reasonable changes like the character limit as you mentioned) and then incremental addition (maybe adding tickets to fully adopt the standard) seems alright. Anyway running on changed files is reasonable since it's better than to run on all files.

And adding this as a template repo also makes sense, for any new projects.

[steven-esser]

@sbs2001 @AyanSinhaMahapatra

Interesting idea about making this repo a GH "template". As long as nothing strange happens to this repo on the GH side once enabled, we can probably adopt this feature.

[steven-esser]

w.r.t. the black question:

I think the real question is do we want to use black or do we use style-check scripts and leave it up to the user to fix issues manually. In the context of this skeleton repo, we have approached from an "automation-first" perspective. The less manual intervention, the better and I believe @JonoYang and @pombredanne agree with me here.

As far as the incremental adoption, remember that we are just discussing in the skeleton repo context. Many existing repos (such as scancode-toolkit) have yet to adopt this internal standard and therefore would not be immediately effected by the adoption of black.

[sbs2001]

@MaJuRG how about using black for both ? black --check as style check in CI and black <whatever> for formatting (which the commiter would need to run).

[steven-esser]

@sbs2001 Good point thanks :) This fits nicely.

[sbs2001]

VulnerableCode finally uses black for both formatting and checking :)

[sbs2001]

VulnerableCode finally uses black for both formatting and checking :)