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.

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

[CI][Python] Lint Python C++ codebase #35485

Closed
wjones127 opened this issue May 8, 2023 · 0 comments · Fixed by #35487
Closed

[CI][Python] Lint Python C++ codebase #35485

wjones127 opened this issue May 8, 2023 · 0 comments · Fixed by #35487

Comments

@wjones127
Copy link
Member

Describe the enhancement requested

It doesn't seem like format checking or linting is run on the Python C++ codebase. Will want to modify archery and potential update docs so contributors know how to fix formatting issues.

Component(s)

Continuous Integration, Developer Tools, Python

@wjones127 wjones127 self-assigned this May 8, 2023
raulcd pushed a commit that referenced this issue May 9, 2023
### Rationale for this change

Python C++ files weren't being formatted. 

### What changes are included in this PR?

This adds a new format to Archery and formats all the files. The last commit contains just the formatting. The first two contain the substantive changes.

Adds a new linter step that activates when both `--python` and `--clang-format` are passed. This was easy to implement and works fine, but does have a disadvantage: You can't run `clang-format` on _just_ the Python C++ codebase. It also runs it on the C++ and that requires a CMake build to be setup. I think this is fine for now.

### Are these changes tested?

Tested in CI and locally. I'd recommend pulling this branch down and verifying you can run this locally as well. We want contributors to be able to run this as well.

### Are there any user-facing changes?

This adds a new linting step. Instructions in the developer documentation are updated to reflect these changes.

* Closes: #35485

Authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
@raulcd raulcd added this to the 13.0.0 milestone May 9, 2023
liujiacheng777 pushed a commit to LoongArch-Python/arrow that referenced this issue May 11, 2023
…che#35487)

### Rationale for this change

Python C++ files weren't being formatted. 

### What changes are included in this PR?

This adds a new format to Archery and formats all the files. The last commit contains just the formatting. The first two contain the substantive changes.

Adds a new linter step that activates when both `--python` and `--clang-format` are passed. This was easy to implement and works fine, but does have a disadvantage: You can't run `clang-format` on _just_ the Python C++ codebase. It also runs it on the C++ and that requires a CMake build to be setup. I think this is fine for now.

### Are these changes tested?

Tested in CI and locally. I'd recommend pulling this branch down and verifying you can run this locally as well. We want contributors to be able to run this as well.

### Are there any user-facing changes?

This adds a new linting step. Instructions in the developer documentation are updated to reflect these changes.

* Closes: apache#35485

Authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this issue May 15, 2023
…che#35487)

### Rationale for this change

Python C++ files weren't being formatted. 

### What changes are included in this PR?

This adds a new format to Archery and formats all the files. The last commit contains just the formatting. The first two contain the substantive changes.

Adds a new linter step that activates when both `--python` and `--clang-format` are passed. This was easy to implement and works fine, but does have a disadvantage: You can't run `clang-format` on _just_ the Python C++ codebase. It also runs it on the C++ and that requires a CMake build to be setup. I think this is fine for now.

### Are these changes tested?

Tested in CI and locally. I'd recommend pulling this branch down and verifying you can run this locally as well. We want contributors to be able to run this as well.

### Are there any user-facing changes?

This adds a new linting step. Instructions in the developer documentation are updated to reflect these changes.

* Closes: apache#35485

Authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this issue May 16, 2023
…che#35487)

### Rationale for this change

Python C++ files weren't being formatted. 

### What changes are included in this PR?

This adds a new format to Archery and formats all the files. The last commit contains just the formatting. The first two contain the substantive changes.

Adds a new linter step that activates when both `--python` and `--clang-format` are passed. This was easy to implement and works fine, but does have a disadvantage: You can't run `clang-format` on _just_ the Python C++ codebase. It also runs it on the C++ and that requires a CMake build to be setup. I think this is fine for now.

### Are these changes tested?

Tested in CI and locally. I'd recommend pulling this branch down and verifying you can run this locally as well. We want contributors to be able to run this as well.

### Are there any user-facing changes?

This adds a new linting step. Instructions in the developer documentation are updated to reflect these changes.

* Closes: apache#35485

Authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment