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

Add black check to Python files in CI #233

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Conversation

anipaul2
Copy link
Contributor

Fixes #230

Description
This PR adds a new job in the CI that checks the formatting of Python files using Black. This ensures that all Python files follow the same code styling guidelines, making the codebase cleaner and easier to read.

Changes

  1. Modified .github/workflows/build.yaml to include a new job for Black checks.

Please let me know if any changes are required.

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

That comment, looks good otherwise.

Cargo.toml Outdated Show resolved Hide resolved
@mariocynicys
Copy link
Collaborator

Also this should be a one commit PR, and sign the commit.

@anipaul2 anipaul2 force-pushed the issue230 branch 2 times, most recently from a6e3293 to 3e263bb Compare July 25, 2023 11:20
@anipaul2
Copy link
Contributor Author

Also this should be a one commit PR, and sign the commit.

I thought of signing when all the checks get successfully passed.

@mariocynicys
Copy link
Collaborator

Rebase on the latest master and give a short commit message like Use black for `.py` files formatting, the commit message you are using right now would be nice as a description instead.

@anipaul2 anipaul2 force-pushed the issue230 branch 2 times, most recently from 45d5b3c to c8f17b4 Compare July 25, 2023 12:29
@anipaul2
Copy link
Contributor Author

Rebase on the latest master and give a short commit message like Use black for `.py` files formatting, the commit message you are using right now would be nice as a description instead.

There is a option which shows update branch and two list which are:-

  1. Update with merge commit
  2. Update with rebase

Are you suggesting me to select the second option?

@mariocynicys
Copy link
Collaborator

There is a option which shows update branch and two list which are

Neither of these. I mean rebasing master on your branch locally and then force pushing on this feature branch.
For this you want to:

  • Make sure you have the remote master up to date: git fetch --all.
  • Rebase on the remote's master: git rebase REMOTE_NAME/master (for instance, I have my REMOTE_NAME as talaia, which points to https://github.com/talaia-labs/rust-teos, you want to pick the remote name you gave for that).
  • Fix any conflicts, I guess in this case there won't be any.
  • Sign the commit you introduced by amending it.
  • Force push, and you will be connected to master.

@anipaul2
Copy link
Contributor Author

There is a option which shows update branch and two list which are

Neither of these. I mean rebasing master on your branch locally and then force pushing on this feature branch. For this you want to:

  • Make sure you have the remote master up to date: git fetch --all.
  • Rebase on the remote's master: git rebase REMOTE_NAME/master (for instance, I have my REMOTE_NAME as talaia, which points to https://github.com/talaia-labs/rust-teos, you want to pick the remote name you gave for that).
  • Fix any conflicts, I guess in this case there won't be any.
  • Sign the commit you introduced by amending it.
  • Force push, and you will be connected to master.

Done. Thanks for the point-to-point explanation.

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

LGTM

.gitignore Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
@anipaul2 anipaul2 force-pushed the issue230 branch 2 times, most recently from c6a9d13 to 150fb74 Compare July 26, 2023 18:59
Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

Some hints

.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Show resolved Hide resolved
@sr-gi
Copy link
Member

sr-gi commented Aug 10, 2023

Needs rebase

@anipaul2 anipaul2 force-pushed the issue230 branch 4 times, most recently from 341305e to cee3764 Compare August 10, 2023 20:41
@sr-gi sr-gi merged commit 6cc466c into talaia-labs:master Aug 21, 2023
7 checks passed
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.

Add black check to .py files in lint CI
3 participants