Page MenuHomePhabricator

[OMPT] Formatting
ClosedPublic

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

Diff Detail

Repository
rOMP OpenMP

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: https://llvm.org/docs/CodingStandards.html#introduction

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
runtime/src/ompt-general.cpp
220–221

Can you also remove this indentation?

Hahnfeld added inline comments.Feb 17 2018, 12:25 AM
runtime/src/ompt-general.cpp
220–221

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.