feat(formatting): 🚀 pre-commmit,pre-commit config,import and code fixes

SystemCraftsman / strimzi-kafka-cli

Command Line Interface for the Strimzi Kafka Operator

Apache License 2.0

78 stars 13 forks source link

feat(formatting): 🚀 pre-commmit,pre-commit config,import and code fixes #111

Closed onuralpszr closed 7 months ago

onuralpszr commented 11 months ago

[X] pre-commit added
[X] pre-commit config yaml added
[X] test fixes
[X] import * fixes and replace with proper import rule
[X] f-string usage
[X] isinstance usage instead of type check
[X] ruff added
[X] generic rules added (white space,end of line space etc)
[X] bandit added
[X] py38 based on rules added
[X] git hook ability
[X] fix #104
[X] fix #106
[X] fix #74
[X] fix #108
[X] fix #105
[X] fix #77

mabulgu commented 11 months ago

Thanks for the pr @onuralpszr ! . Let me check pr #110 first (probably this weekend, sorry for the latency) and then checkout this one. I am a bit obsessed with formatting and I dont want to apply all the pep8 or other derived rules. Lets merge your first pr first and then I need to go over this one and we can discuss afterward.

Thanks for the great work!

onuralpszr commented 11 months ago

mabulgu commented 11 months ago

https://github.com/SystemCraftsman/strimzi-kafka-cli/issues/77

@onuralpszr you mentioned you fix this one with this PR. Since the PR is big, before my detailed look at this (and #110 should come first) can I ask where and how exactly did you solve this. I am just asking not to miss anything.

onuralpszr commented 11 months ago

77

@onuralpszr you mentioned you fix this one with this PR. Since the PR is big, before my detailed look at this (and #110 should come first) can I ask where and how exactly did you solve this. I am just asking not to miss anything.

Merge #110 and if we change something I can rebase or fix conflict in here. But if you accept "as it is" well good because I create this PR from #110. Magical solve will happen :))

mabulgu commented 11 months ago

77

@onuralpszr you mentioned you fix this one with this PR. Since the PR is big, before my detailed look at this (and #110 should come first) can I ask where and how exactly did you solve this. I am just asking not to miss anything.

Merge #110 and if we change something I can rebase or fix conflict in here. But if you accept "as it is" well good because I create this PR from #110. Magical solve will happen :))

Can you explain the magical solve for #77 if you dont mind? Not sure this one is fixed.

mabulgu commented 11 months ago

And yes #110 should be merged first as you have #110 in this one as well (so you will need a rebase for sure)

onuralpszr commented 11 months ago

77

@onuralpszr you mentioned you fix this one with this PR. Since the PR is big, before my detailed look at this (and #110 should come first) can I ask where and how exactly did you solve this. I am just asking not to miss anything.

Merge #110 and if we change something I can rebase or fix conflict in here. But if you accept "as it is" well good because I create this PR from #110. Magical solve will happen :))

Can you explain the magical solve for #77 if you dont mind? Not sure this one is fixed.

https://github.com/SystemCraftsman/strimzi-kafka-cli/pull/111/files#diff-6c5cdeeef00512ffa0304dbc3fb40ce1cd56276f4671b3d78994b5d8668a116aR12

no command gives you "help" but I guess you want "kfk command" <-- no param == shows you help ?

mabulgu commented 11 months ago

Hi @onuralpszr. Would you mind separating this PR into two like 1- pre-commit and other stuff like fstring usage etc. 2- formatting

So it will be a lot easier to review and discuss the changes.

Would you be able to do this? If not, pls let me know so I can work on this.