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

Modify formatter version in contributing guide #5509

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

Dbhasin1
Copy link
Contributor

@Dbhasin1 Dbhasin1 commented Mar 2, 2022

@facebook-github-bot
Copy link

facebook-github-bot commented Mar 2, 2022

💊 CI failures summary and remediations

As of commit b9a7842 (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@@ -83,7 +83,7 @@ Instead of relying directly on `black` however, we rely on
[ufmt](https://github.com/omnilib/ufmt), for compatibility reasons with Facebook
internal infrastructure.

To format your code, install `ufmt` with `pip install ufmt` and use e.g.:
To format your code, install `ufmt` with `pip install ufmt==1.3.2 black==21.9b0 usort==0.6.4` and use e.g.:
Copy link
Contributor

@oke-aditya oke-aditya Mar 2, 2022

Choose a reason for hiding this comment

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

I think ufmt automatically installs black and usort? Or is it that it does not pin these versions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ufmt only has very lose pins: https://github.com/omnilib/ufmt/blob/a5c73d35bcc00a99f2cfbb7f12eb15ef340db9b6/pyproject.toml#L11-L17

Since we check against these versions in CI, we need a strict pin.

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks a lot @Dbhasin1!

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM!

@datumbox datumbox merged commit cef22ea into pytorch:main Mar 2, 2022
@datumbox datumbox added the other if you have no clue or if you will manually handle the PR in the release notes label Mar 2, 2022
@github-actions
Copy link

github-actions bot commented Mar 2, 2022

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

facebook-github-bot pushed a commit that referenced this pull request Mar 5, 2022
Summary: Co-authored-by: Dbhasin1 <drishti_b@me.iitr.c.in>

Reviewed By: datumbox

Differential Revision: D34579509

fbshipit-source-id: c32ed808e31d57c0d9c511a8d2b85c2aa1a4ac63
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/default cla signed other if you have no clue or if you will manually handle the PR in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants