1535798 - [Automated review] clang-format bot suggests edits to parts of the file that were not changed and a hunk that doesn't apply

Status: RESOLVED INVALID

(Developer Infrastructure :: Source Code Analysis, defect, P1)

Description

[(no longer active)]

Phabricator URL: https://phabricator.services.mozilla.com/D23616

Here is the diff: https://taskcluster-public-blobs.s3.amazonaws.com/AZzXMEz0T7CWPUL1nwPiug/0/public/patch/clang-format-PHID-DIFF-orqfiytxg4wojujaexia.diff

The HttpBaseChannel.cpp changes are parts of the file this patch did not change. And I don't understand the InputStreamLengthHelper.h change... As far as I can tell the existing code in the patch is correctly formatted and in fact that hunk should not even apply!

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Andi, as the log are still available: could you please have a look quickly? (it can wait until next Monday) Thanks

Comment 2

[Andi [:andi]]

Ehsan thank you for reporting this issue, for the moment our bot doesn't support a per-hunk filtering when it comes to clang-format that's why we don't even post in-line comments like we do for example with coverity and clang-tidy, right now we only do a per-file format and we post a diff.

When it comes to InputStreamLengthHelper.h, clang-format modified the last line of the patch by deleting one space and adding two spaces for the comment of #endif, do you think this is something wrong? Do you think we should keep only once space?

Comment 3

[(no longer active)]

Hi Andi,

Sorry for my delay here.

About InputStreamLengthHelper.h, I think you are right, and I must have been confused before. I can't even remember what I was thinking at the time I filed the bug but looking at things again indeed clang-format was just doing its job, so perhaps my brain failed me there for a moment. :-) Please ignore that part of my comment.

Comment 4

[Andi [:andi]]

Ehsan are you OK closing this bug?

Comment 5

[(no longer active)]

Sure.