Style and formatting rules

[jacobgarder]

Style, formatting can always can be a religious thing, but when we started to discuss it, I just put in my 50 cents 😀.
For me it is no big deal, but I think what ever rules are set, they should as clear as possible and there were some things I found a bit unclear when I started to contribute to this project that I more or less have guessed.

So Mitch, please correct me if I'm wrong:

• Formatting is done with Black
• Indentation is done with spaces (4) and not tabs (Black)
• Line-length is set to 127 (matching github width)
• Flake8 is used for linting in the pipeline.
• Pylint is used (to some extent)

Apart from this, I also think personally that:

• Imports should be sorted (with isort)
• Import should be absolute (not reative) as this is more clear and I have come across IDEs that don't work very well with them (but that was a long time ago)

In this PR I have included configuration for these formatting tools which makes it easier for all contributors to conform to the formatting rules and updated documentation abit.

I have also reformatted the files accordingly, even the docsrc and tests directory...this could be argued if they should be included or not..

[mitchos]

So Mitch, please correct me if I'm wrong:

Formatting is done with Black 
Indentation is done with spaces (4) and not tabs (Black) 
Line-length is set to 127 (matching github width) 
Flake8 is used for linting in the pipeline. 
Pylint is used (to some extent) 

Apart from this, I also think personally that:

Imports should be sorted (with isort) 
Import should be absolute (not reative) as this is more clear and I have come across IDEs that don't work very well with them (but that was a long time ago)

My thoughts

✔️ Happy with all of these choices.

1. Wow I didn't know about isort, that is really handy. I actually wondered if there was something that did this so there we go.
2. If you've found some cases where this doesn't work then I don't have a problem moving to absolute. It was more something that I just did for convenience rather than putting any initial thought into it.

So here are my thoughts on absolute vs relative for this case:

• pyZscaler is pretty compact and the relative imports aren't long enough to be 'messy' or 'inconvenient' yet
• I don't think anyone will be making such dramatic changes to that using absolute would cause any heartache
• Absolute is still readable and doesn't detract
• I can't think of / foresee any case where absolute wouldn't work but as you've highlighted, there's certainly potential issues using relative.

So with that ✔️ happy to start moving across to absolute.

Thanks again for this PR, there's some great stuff here that's contributing to the overall quality.

• One last thing, do you mind just making a minor change (potentially fixing the imports in ZIA/ZPA init). I forgot to put develop on the build Workflow and the tests aren't automatically running. I've updated it and hoping it will kick off the tests if you push another commit

[mitchos]

@daxm just wanted to see if you had any thoughts on style while we're discussing?

[jacobgarder]

* [ ]  One last thing, do you mind just making a minor change (potentially fixing the imports in ZIA/ZPA init). I forgot to put develop on the build Workflow and the tests aren't automatically running. I've updated it and hoping it will kick off the tests if you push another commit

Done.

[daxm]

I love Black.
I've used flake8 in the past and other than 1 or 2 things that it nagged me about I can't complain.
I also wasn't aware of isort. Excited to see how it works out.
I have no preference on full vs relative import paths.

Jacob,
I noticed that Mitch and I both are unfamiliar with a few of these suggestions you have. The only "complaint"/"concern" that I can think of is that having a lot of "requirements" might scare off potential contributors. Could you write up a "howto" doc on how a potential contributor should setup their environment and what checks they should run prior to a PR?
I used to have a "prepare_to_deploy.sh" file in my projects which I would run to ensure I met things like you list above.

[jacobgarder]

I love Black. I've used flake8 in the past and other than 1 or 2 things that it nagged me about I can't complain.

I agree, Black makes life a lot easier. And I also think Mitch did a good choice in extending the line-length.

Jacob, I noticed that Mitch and I both are unfamiliar with a few of these suggestions you have. The only "complaint"/"concern" that I can think of is that having a lot of "requirements" might scare off potential contributors. Could you write up a "howto" doc on how a potential contributor should setup their environment and what checks they should run prior to a PR? I used to have a "prepare_to_deploy.sh" file in my projects which I would run to ensure I met things like you list above.

I think the only addition really is isort, the rest of the tests are already in place in the workflow, configured my Mitch I assume. I just tried to state them so that new contributors do not need to guess.
To write an exact howto is hard if you do not control the contributors environment; what IDE to use, and how they manage the python env. (venv / / pyenv / pipenv / docker dev environment). Having said that, I think its a good idea to do these checks in advance as you say. I've usually use git pre-commit hooks for that purpose. Have you tried it?
I would say the pros are that you do not need to remember to run the "format/lint-script" before committing as git will do it for you.
The cons are that if you are not used to it, it can be a bit confusing that as the commit fails if some file does not pass the checks. If you want, I can make a suggestion on how a .precommit-config should look like and a short "howto" on how to install it. An alternative is to provide a "prepare_to_depoly" script in the repo, which also can be an idea. What do you think?

[daxm]

a .precommit-config would be great, for me. I'm sure that I'm not the only one who is not familiar with that. And, yes, if a commit fails without a user understanding would be VERY frustrating. Anything we can do to inform the user WHY it failed would be great in my book.

[jacobgarder]

I have added .pre-commit-config.yaml.
To use it, you need to install in your environment by installing pre-commit package and also install the pre-commit hooks (in .git/hooks/pre-commit directory). This is done with the following commands:

pip install pre-commit
pre-commit install

[mitchos]

Can you just review those two comments and I'll approve this after.

Thanks again for your work on this one 😃

[jacobgarder]

Thanks! 😃
Added the changes you requested.

[mitchos]

Looks good to me. Happy we're moving forward with this.