Skip to content

add python code style lint and format#147

Merged
li-wu merged 1 commit intodevelopfrom
codeformat
Mar 21, 2019
Merged

add python code style lint and format#147
li-wu merged 1 commit intodevelopfrom
codeformat

Conversation

@li-wu
Copy link
Copy Markdown
Contributor

@li-wu li-wu commented Mar 20, 2019

No description provided.


build_spl: clean
python -m splunk_eventgen build --destination ./

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make lint will only lint the newly added and changed parts of python files here.

@flake8 $(NEWLY_ADDED_PY_FILES) || true
endif
@git diff -U0 -- '*.py' | flake8 --diff || true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make format will only format newly added python files and sort imports for both newly added and changed python files.

description-file = README.md
version-from-file: __init__.py

[yapf]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yapf style is based on pep8 here.

@arctan5x
Copy link
Copy Markdown
Contributor

@li-wu Will you be including linted code changes to this PR?

@li-wu
Copy link
Copy Markdown
Contributor Author

li-wu commented Mar 20, 2019

@arctan5x , not really. This PR will not include the formatting changes for existing Python code. You can see that the command make lint and make format only apply to newly added or changed Python files.
My thought is after we have added enough test cases for existing code, we have a separate PR for the code formatting. Another option is when we fix bug or add new features, we lint/format the code along with the changes. Any other suggestions?

@arctan5x
Copy link
Copy Markdown
Contributor

@li-wu If you want to introduce linting, I would recommend applying a linter to the codebase all together after the tests cases are added.

@GordonWang
Copy link
Copy Markdown
Contributor

@li-wu If you want to introduce linting, I would recommend applying a linter to the codebase all together after the tests cases are added.

Totally agree!
We should do this in one commit.

Copy link
Copy Markdown
Contributor

@GordonWang GordonWang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you all.
Let's format all the code after adding the unit test cases. In case there is any bug in the formatting tool which may introduce regression after formatting all the code.

@li-wu li-wu merged commit eacc92d into develop Mar 21, 2019
@li-wu li-wu deleted the codeformat branch April 19, 2019 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants