[Differential] [Commented On] D4589: EditorConfig module

2017-02-14 Thread Dominik Haumann 
dhaumann added inline comments.

INLINE COMMENTS

> Findeditorconfig.cmake:2
> +#
> +# Copyright (c) 2016 João Valverde 
> +# All rights reserved.

1. Please add yourself as copyright holder.

2. Could you have a look at the comments at the very top of the other find 
modules in the extra-cmake-modules? There, you can find a ".rst" header that 
contains documentation about the find module, see e.g. libgit2 again. Could you 
add this documentation as well for this find module?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D4589

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #build_system, #frameworks, alexmerry
Cc: dhaumann

[Differential] [Commented On] D4589: EditorConfig module

2017-02-13 Thread Dominik Haumann 
dhaumann added a comment.


  Hm, looking at libgit2 I have:
  
-rwxr-xr-x 1 root root 906728 Dez  2 12:13 libgit2.so.0.24.0
lrwxrwxrwx 1 root root 17 Jan  7 20:05 libgit2.so.24 -> 
libgit2.so.0.24.0
  
  Looking at editorconfig, I have:
  
lrwxrwxrwx 1 root root20 Feb 12 17:17 libeditorconfig.so -> 
libeditorconfig.so.0
lrwxrwxrwx 1 root root25 Feb 12 17:17 libeditorconfig.so.0 -> 
libeditorconfig.so.0.12.1   
   
-rwxr-xr-x 1 root root 18320 Aug 17 12:42 libeditorconfig.so.0.12.1 

 
  
  So the shared library definitely has a version number. I cannot say anything 
with respect to binary compatibility guarantees of the editorconfig library, 
but in general, checking for a version is a good idea :-)
  
  I think we need input from developers who know cmake and the 
extra-cmake-modules here.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D4589

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #build_system, #frameworks, alexmerry
Cc: dhaumann

[Differential] [Commented On] D4589: EditorConfig module

2017-02-13 Thread Grzegorz Szymaszek 
gszymaszek added a comment.


  editorconfig-core-lua  
doesn’t check C library version (CMakeLists.txt:49 
).
 Actually I’m not sure if setting oldest acceptable version in KTextEditor is 
really needed.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D4589

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #build_system, #frameworks, alexmerry
Cc: dhaumann

[Differential] [Commented On] D4589: EditorConfig module

2017-02-13 Thread Grzegorz Szymaszek 
gszymaszek added a comment.


  It seems there isn’t any version string in library headers (editorconfig.h, 
editorconfig_handle.h), is there any other cross-distribution way to obtain 
library version?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D4589

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #build_system, #frameworks, alexmerry
Cc: dhaumann

[Differential] [Commented On] D4589: EditorConfig module

2017-02-12 Thread Dominik Haumann 
dhaumann added a comment.


  Correct is that we want to write
  
find_package(editorconfig "0.12.0)
  
  i.e., with a version. This is missing in this find module, see the LibGit2 
find module. Could you extend this?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D4589

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: gszymaszek, #build_system, #frameworks
Cc: dhaumann