Closed Bug 1530698 Opened 5 years ago Closed 5 years ago

[Automated review] Consider having reviewbot provide more actionable clang-format feedback

Categories

(Developer Infrastructure :: Source Code Analysis, enhancement, P2)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gregtatum, Assigned: andi)

References

Details

Reviewbot does not provide a very actionable message for clang-format. It is confusing because it gives a command that does not point to a real file. I would prefer a single command that I could copy, paste, and run from my terminal to fix the problem. For example, right now it lists:

./mach clang-format -s -p path/to/file.cpp

This command is confusing for two reasons:

  1. The path/to/file.cpp is not a real file, but looks like one.
  2. The command outputs a patch, but does not actually fix the problem.

At this point I have no single command I can run to just fix the problem.

I would prefer this command listed:

./mach clang-format -p toolkit/xre/nsEmbedFunctions.cpp
./mach clang-format -p js/xpconnect/src/XPCShellImpl.cpp

Or perhaps a one liner for the hg import command.

Severity: normal → enhancement
Priority: -- → P2

Good points, thanks!

Blocks: clang-format

How about just suggesting ./mach clang-format (no args), as it works on a whole patch?

I believe ./mach clang-format only works on uncommitted work. By the time a patch reaches phabricator, the code has already been committed, so it won't reformat the offending files.

Well, you'll be glad to learn that in fact it works on both uncommitted changes (if any) and the already-committed patch you're at. 😉

Andi, could you please take care of that?
Thanks

Assignee: nobody → bpostelnicu
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.