Page MenuHomePhabricator

[scan-build-py] fix some line separator issues
ClosedPublic

Authored by rizsotto.mailinglist on Mar 3 2017, 8:16 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

dcoughlin accepted this revision.Mar 5 2017, 1:09 PM

Looks good to me, although please update the comment to be prose and have proper capitalization.

Also: it really helps to have patches with context -- it makes patches much faster to review. (See http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface )

This revision is now accepted and ready to land.Mar 5 2017, 1:09 PM

Looks good to me, although please update the comment to be prose and have proper capitalization.

Can I ask a temporary exemption on capitalization? Here's why I'm asking... Currently all pydoc comments are capitalized, but inline ones are not. So, it's kind of consistent this way. (I understand that's not what the project guidelines says.) And I would like to catch up with the changes I have on my github project. It will take 20-30 more patches like this. It's much easier to do this if I can diff the two source tree... And when I'm done with it, will make a patch on capitalization only, where we can fix my bad English, or reformulate comments when needed.

Also: it really helps to have patches with context -- it makes patches much faster to review. (See http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface )

Sure, will do more context. Have not checked the generated diff, sorry.

Looks good to me, although please update the comment to be prose and have proper capitalization.

Can I ask a temporary exemption on capitalization? Here's why I'm asking... Currently all pydoc comments are capitalized, but inline ones are not. So, it's kind of consistent this way. (I understand that's not what the project guidelines says.) And I would like to catch up with the changes I have on my github project. It will take 20-30 more patches like this. It's much easier to do this if I can diff the two source tree... And when I'm done with it, will make a patch on capitalization only, where we can fix my bad English, or reformulate comments when needed.

I think skipping the capitalization for now is perfectly fine. But we do need comments in prose, otherwise we can't tell what the intent of the upstreamed patch is and so can't review it properly.

This revision was automatically updated to reflect the committed changes.