ci/lint: fix calling Ruff's format

[Borda]

It seems that Ruff'f format was not called correctly; in fact, as both Ruff's IDs had overwritten entry, it was still calling check with a confusion of id: ruff-format

Found based on https://github.com/OpenDevin/OpenDevin/pull/1425#discussion_r1582056596

See the original entry call: https://github.com/astral-sh/ruff-pre-commit/blob/596470fba20d04adc68ec7903ff69a12e5e1a8d3/.pre-commit-hooks.yaml#L15

cc: @rbren @li-boxuan

[codecov-commenter]

Welcome to Codecov :tada:

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered :open_umbrella:

[Borda]

LGTM, but it's gonna be a huge pain for people to handle merge conflicts :(

Yeah, only other way is drop false hook with "formatting"

[rbren]

@Borda I'm generally a fan of enforcing new style changes one by one as they change--is that possible here?

[Borda]

I'm generally a fan of enforcing new style changes one by one as they change--is that possible here?

@rbren Not sure what you mean, this is Black.

[li-boxuan]

I'm generally a fan of enforcing new style changes one by one as they change--is that possible here?

It should be possible. We just need to run pre-commit only on changed files (i.e. on diff), not all files.

@Borda the ask is, can we avoid linting all files in this PR? This is a huge PR that will cause a lot of open PRs to have merge conflicts.

[Borda]

can we avoid linting all files in this PR? This is a huge PR that will cause a lot of open PRs to have merge conflicts.

Yes, we can, just to be aware that many PRs will include unrelated changes to the PR's topic... For example, simple PR with fixing typo may suddenly include another 50 changes

[li-boxuan]

For example, simple PR with fixing typo may suddenly include another 50 changes

I think that's fine. It definitely reduces the burden for developers. People hate merge conflicts and if they hate it too much, they will start to hate all linting efforts.

[Borda]

@li-boxuan @rbren dropped applied pre-commit but noted that the lining job will fail...

[iFurySt]

pre-commit run --files $(git diff --name-only $(git merge-base main $(git branch --show-current)) $(git branch --show-current) | tr '\n' ' ') ...

this method can ensure only checking the modified files. but it is a bit troublesome to apply to workflow. it needs to fetch the main and current branch and unshallow clone~

[iFurySt]

@rbren @li-boxuan what do you think of this method to transition?

[iFurySt]

LGTM!

it causes the lint workflow to fail right now 🤔

[li-boxuan]

Love this approach

Get Outlook for iOShttps://aka.ms/o0ukef

---

From: Leo @.> Sent: Wednesday, May 1, 2024 4:53 AM To: OpenDevin/OpenDevin @.> Cc: @.*** @.>; Mention @.> Subject: Re: [OpenDevin/OpenDevin] ci/lint: fix calling Ruff's format (PR #1457)

pre-commit run --files $(git diff --name-only $(git merge-base main $(git branch --show-current)) $(git branch --show-current) | tr '\n' ' ') ...

this method can ensure only checking the modified files. but it is a bit troublesome to apply to workflow. it needs to fetch the main and current branch and unshallow clone~

— Reply to this email directly, view it on GitHubhttps://github.com/OpenDevin/OpenDevin/pull/1457#issuecomment-2088348022, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGENUWUVD5ON4IGEPRIWDF3ZADJS5AVCNFSM6AAAAABG7GJ5MOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBYGM2DQMBSGI. You are receiving this because you were mentioned.Message ID: @.***>

[rbren]

Ugh you're right. Not sure how we did this in the past. Maybe we do just need to delint everything in this PR and deal with the merge headaches

[li-boxuan]

In the past we don’t have (enough) linting checks in CI.

How about dropping the CI enforcement for a while?

Get Outlook for iOShttps://aka.ms/o0ukef

---

From: Robert Brennan @.> Sent: Wednesday, May 1, 2024 8:44:02 AM To: OpenDevin/OpenDevin @.> Cc: @.*** @.>; Mention @.> Subject: Re: [OpenDevin/OpenDevin] ci/lint: fix calling Ruff's format (PR #1457)

Ugh you're right. Not sure how we did this in the past. Maybe we do just need to delint everything in this PR and deal with the merge headaches

— Reply to this email directly, view it on GitHubhttps://github.com/OpenDevin/OpenDevin/pull/1457#issuecomment-2088656141, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGENUWVJ47SNEIAN54APTGTZAEEUFAVCNFSM6AAAAABG7GJ5MOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBYGY2TMMJUGE. You are receiving this because you were mentioned.Message ID: @.***>

[iFurySt]

I've pushed a patch for transition. It only checks the modified files from the commits.