Page MenuHomePhabricator

[OMPT] Formatting

Authored by sconvent on Jan 17 2018, 5:15 AM.

Diff Detail


Event Timeline

sconvent created this revision.Jan 17 2018, 5:15 AM

Hmm, this should generally be done for each change and not for the sake of formatting:

but we explicitly do not want patches that do large-scale reformatting of existing code.

Actually the patch looks acceptable to me, as it is not a "large-scale reformatting" but only 6 lines of code.

Honestly I don't really care, but please keep the indention in ompt_try_start_tool or move the comment so that clang-format doesn't get it wrong.

sconvent updated this revision to Diff 134190.Feb 14 2018, 3:32 AM

Exclude formatting of ompt_try_start_tool

hbae added inline comments.Feb 16 2018, 12:44 PM

Can you also remove this indentation?

Hahnfeld added inline comments.Feb 17 2018, 12:25 AM

I explicitly requested this change to be reverted. In my opinion the comment really belongs to this level as it relates to all function calls in the #if blocks. That said we might want to put it somewhere so that clang-format doesn't get it wrong.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 17 2018, 1:57 AM
Closed by commit rOMP325424: [OMPT] Omissionin in OMPT Formatting (authored by jprotze, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.