search

banach-space / clang-tutor

A collection of out-of-tree Clang plugins for teaching and learning
The Unlicense
659 stars 62 forks source link

[NFC] Fix minor typos in Readme #14

Closed xgupta closed 3 years ago

banach-space commented 3 years ago

Thanks a ton for submitting this! Quite embarrassing how much I missed!

Is -DCMAKE_EXPORT_COMPILE_COMMANDS=ON really needed there? Personally I prefer to keep the description of HelloWorld to the required minimum. But I might be missing something here!

-Andrzej

xgupta commented 3 years ago

Thanks @banach-space for reviewing! please merge the PR.

banach-space commented 3 years ago

Hm, IIUC all changes related to CMAKE_EXPORT_COMPILE_COMMANDS can be skipped.

Basically, either we require CMAKE_EXPORT_COMPILE_COMMANDS (which generates compile_commands.json) or all tools should be run with -- at the end, e.g.:

<build_dir>/bin/ct-code-refactor --class-name=Base --new-name=bar --old-name=foo file.cpp --

I think that the latter is cleaner as ct-code-refactor won't use ompile_commands.json anyway.

Sorry for not making it clearer in my earlier comment.

banach-space commented 3 years ago

Would you mind squashing? Then I can merge. I don't want to rely on GitHub's UI (and I want to avoid merge commits too). Thanks a bunch!

Edit: basically, rebase+squash and then force push to your own branch so that there's only one commit with all your changes.

xgupta commented 3 years ago

Sorry for not making it clearer in my earlier comment.

No problem -:)

banach-space commented 3 years ago

Your patience is much appreciated! :)