Re: [cmake-developers] Two pull requests
2012/3/1 Brad King : > On 3/1/2012 10:47 AM, Eric Noulard wrote: >> >> Last time I tried to enable KWStyle hooks following this: >> http://www.cmake.org/Wiki/Git/Hooks#Setup > > > That page is generic for many of Kitware's projects and > is not specific to CMake. The same hooks are also used > for ITK. For a while the kwstyle and uncrustify hooks > were added and used by ITK. Later they were moved over > to ITK proper and are now invoked through the generic > hooks' chaining feature. They no longer exist in the > generic hooks outright. I removed the discussion of > them from the wiki page. > > I'm not a big fan of automatic code formatting and > layout tools. There are almost always exceptions and > special cases. I'd rather cover that during code review. Yes I agree with that but may be using them as pre-commit "warning" on changed files may be interesting. Nevertheless checking for no more than 79 column style certainly does not require such tool. -- Erk Le gouvernement représentatif n'est pas la démocratie -- http://www.le-message.org -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
On 3/1/2012 10:47 AM, Eric Noulard wrote: Last time I tried to enable KWStyle hooks following this: http://www.cmake.org/Wiki/Git/Hooks#Setup That page is generic for many of Kitware's projects and is not specific to CMake. The same hooks are also used for ITK. For a while the kwstyle and uncrustify hooks were added and used by ITK. Later they were moved over to ITK proper and are now invoked through the generic hooks' chaining feature. They no longer exist in the generic hooks outright. I removed the discussion of them from the wiki page. I'm not a big fan of automatic code formatting and layout tools. There are almost always exceptions and special cases. I'd rather cover that during code review. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
2012/3/1 Brad King : > On 2/29/2012 4:58 PM, Yury G. Kudryashov wrote: > >> Brad King wrote: >>> >>> Our style checker limits .h and .cxx files to 79 columns. >>> Some of the updated comments exceed this limit. Please >>> reformat them. >> >> Where can I find the style checker sources? I'll run it to pre-commit hook >> then. > > > The CMAKE_USE_KWSTYLE option enables it but you need to build > the KWStyle tool. Our dashboard also runs it on the repository > to catch post-commit failures. > > However, the *only* check it currently performs is the column > limit AFAIK. Someday I may get around to adding the check in > our pre-commit hook. Last time I tried to enable KWStyle hooks following this: http://www.cmake.org/Wiki/Git/Hooks#Setup It doesn't seem to work. I did setup my emacs/eclipse to show post 79 characters instead. That said may be sharing some "uncrustify" config file would be nice. kwstyle, uncrustify, astyle etc... are usually found easily on distro repository or downloadable (don't know for KWStyle) as windows binary and may be MacOS DMG as well. -- Erk Le gouvernement représentatif n'est pas la démocratie -- http://www.le-message.org -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
On 2/29/2012 5:34 PM, Yury G. Kudryashov wrote: Our style checker limits .h and .cxx files to 79 columns. Some of the updated comments exceed this limit. Please Force-pushed. Thanks. It conflicts with the add-const-qualifiers topic in cmPropertyDefinition::IsChained. I merged that into this topic to resolve it. Then I merged to 'next' for testing. BTW, what is the policy for const MyClass* vs MyClass const*? I personally prefer the latter except for "const char*" because it is in such common use. We don't have a strict requirement. Which parts of STL are allowed in sources that are needed for bootstrap? There is no special restriction for bootstrap v. main build that I remember. Any of the basic containers are allowed. We tend to avoid the algorithms beyond any already in use because many older STL implementations where CMake builds are limited. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
On 2/29/2012 4:58 PM, Yury G. Kudryashov wrote: Brad King wrote: Our style checker limits .h and .cxx files to 79 columns. Some of the updated comments exceed this limit. Please reformat them. Where can I find the style checker sources? I'll run it to pre-commit hook then. The CMAKE_USE_KWSTYLE option enables it but you need to build the KWStyle tool. Our dashboard also runs it on the repository to catch post-commit failures. However, the *only* check it currently performs is the column limit AFAIK. Someday I may get around to adding the check in our pre-commit hook. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
Brad King wrote: > On 2/28/2012 4:21 PM, Brad King wrote: >> On 2/28/2012 4:02 PM, Yury G. Kudryashov wrote: >>> git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git >>> run-vim-spellcheck > > Merged, thanks: > > http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=adc2c990 > >>> git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git doxygen-fixes > > Our style checker limits .h and .cxx files to 79 columns. > Some of the updated comments exceed this limit. Please Force-pushed. BTW, what is the policy for const MyClass* vs MyClass const*? Which parts of STL are allowed in sources that are needed for bootstrap? -- Yury G. Kudryashov, mailto: ur...@mccme.ru -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
Brad King wrote: > On 2/28/2012 4:21 PM, Brad King wrote: >> On 2/28/2012 4:02 PM, Yury G. Kudryashov wrote: >>> git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git doxygen-fixes > > Our style checker limits .h and .cxx files to 79 columns. > Some of the updated comments exceed this limit. Please > reformat them. Where can I find the style checker sources? I'll run it to pre-commit hook then. -- Yury G. Kudryashov, mailto: ur...@mccme.ru -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
On 2/28/2012 4:21 PM, Brad King wrote: On 2/28/2012 4:02 PM, Yury G. Kudryashov wrote: git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git run-vim-spellcheck Merged, thanks: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=adc2c990 git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git doxygen-fixes Our style checker limits .h and .cxx files to 79 columns. Some of the updated comments exceed this limit. Please reformat them. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
On 2/28/2012 4:02 PM, Yury G. Kudryashov wrote: git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git run-vim-spellcheck git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git doxygen-fixes Thanks. I'll look at those topics when I get a chance. We accept any form of patch that comes in if it is easy to apply. Fetching real Git commits is a nice way to get them :) It seems that merge requests are disabled in repository settings. That's intentional, so in that sense we do not support first-class pull requests. We want requests to come through this mailing list. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
Brad King wrote: > On 2/28/2012 2:46 PM, Yury G. Kudryashov wrote: >> git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git >> ready/fix-typos > > Please combine those commits and write a single commit message > that briefly explains the tools you ran to find the errors. I've also renamed branch. git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git run-vim- spellcheck > >> Next, I fixed some doxygen formatting. >> git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git ready/apidocs- >> fixes > > Please prefix the commit messages with "doxygen: " since all the > changes are for Doxygen comment formatting. Done, new branch: git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git doxygen-fixes > >> The last commit in this branch replaces all '///!' by '///'. This can >> potentially conflict with many topic branches. One of the ways to avoid >> such conflicts is to run `sed -e 's,///!,///' -i *.h` in each branch that >> conflicts with apidocs-fixes branch. > > I'd rather make that as a sweeping change and then rebase active > topics on it. This is better done right after a release when > there are no major topics underway. Let's drop that commit > for now. OK, dropped. > >> P.S.: Do you accept gitorious pull requests, or should I send e-mails >> like this? I don't want to fill a bug report in mantis for each small >> fix. > > We accept any form of patch that comes in if it is easy to apply. > Fetching real Git commits is a nice way to get them :) It seems that merge requests are disabled in repository settings. -- Yury G. Kudryashov, mailto: ur...@mccme.ru -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Two pull requests
On 2/28/2012 2:46 PM, Yury G. Kudryashov wrote: > I've published two branches on gitorious. Thanks for your work! First, I run spellcheck on some source files. git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git ready/fix-typos Please combine those commits and write a single commit message that briefly explains the tools you ran to find the errors. Next, I fixed some doxygen formatting. git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git ready/apidocs- fixes Please prefix the commit messages with "doxygen: " since all the changes are for Doxygen comment formatting. The last commit in this branch replaces all '///!' by '///'. This can potentially conflict with many topic branches. One of the ways to avoid such conflicts is to run `sed -e 's,///!,///' -i *.h` in each branch that conflicts with apidocs-fixes branch. I'd rather make that as a sweeping change and then rebase active topics on it. This is better done right after a release when there are no major topics underway. Let's drop that commit for now. P.S.: Do you accept gitorious pull requests, or should I send e-mails like this? I don't want to fill a bug report in mantis for each small fix. We accept any form of patch that comes in if it is easy to apply. Fetching real Git commits is a nice way to get them :) Thanks! -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
[cmake-developers] Two pull requests
Hi! I've published two branches on gitorious. First, I run spellcheck on some source files. git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git ready/fix-typos Next, I fixed some doxygen formatting. git pull git://gitorious.org/~urkud1/cmake/urkud-cmake.git ready/apidocs- fixes The last commit in this branch replaces all '///!' by '///'. This can potentially conflict with many topic branches. One of the ways to avoid such conflicts is to run `sed -e 's,///!,///' -i *.h` in each branch that conflicts with apidocs-fixes branch. P.S.: Do you accept gitorious pull requests, or should I send e-mails like this? I don't want to fill a bug report in mantis for each small fix. -- Yury G. Kudryashov, mailto: ur...@mccme.ru -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
