Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add python code style lint and format #147

Merged
merged 1 commit into from
Mar 21, 2019
Merged

add python code style lint and format #147

merged 1 commit into from
Mar 21, 2019

Conversation

li-wu
Copy link
Contributor

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

No description provided.

li-wu
li-wu commented Mar 20, 2019
View reviewed changes
Makefile
@@ -101,3 +103,24 @@ docs:

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

Copy link
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.

li-wu
li-wu commented Mar 20, 2019
View reviewed changes
Makefile
@flake8 $(NEWLY_ADDED_PY_FILES) || true
endif
@git diff -U0 -- '*.py' | flake8 --diff || true

Copy link
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.

li-wu
li-wu commented Mar 20, 2019
View reviewed changes
setup.cfg
max-line-length = 120

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

[yapf]
Copy link
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
Contributor

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

@li-wu
Copy link
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
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
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.

GordonWang
GordonWang approved these changes Mar 20, 2019
View reviewed changes
Copy link
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
Reviewers

@GordonWang GordonWang GordonWang approved these changes

@arctan5x arctan5x arctan5x approved these changes

@lephino lephino Awaiting requested review from lephino

@jmeixensperger jmeixensperger Awaiting requested review from jmeixensperger

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@li-wu @arctan5x @GordonWang