Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pkgconfig support #36

Merged
merged 1 commit into from
Mar 15, 2018
Merged

Pkgconfig support #36

merged 1 commit into from
Mar 15, 2018

Conversation

tintou
Copy link

@tintou tintou commented Mar 15, 2018 •
edited
Loading

No description provided.

@tintou tintou changed the title Pkgconfig Pkgconfig support Mar 15, 2018
xuhdev
xuhdev requested changes Mar 15, 2018
View reviewed changes
src/lib/editorconfig.pc.in Outdated
includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@/editorconfig

Name: editorconfig
Description: Configuration file format for defining coding styles in shared projects
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line may be confusing -- the package itself is not a configuration file. Maybe

EditorConfig C Core: A library for handling EditorConfig files, a configuration file format for defining coding styles in shared projects.

Do you think editorconfig-core-c is a better name?

CMakeLists.txt Outdated
cmake_minimum_required(VERSION 2.8.7)
cmake_policy(VERSION 2.8.7)
cmake_minimum_required(VERSION 3.0)
cmake_policy(VERSION 3.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change necessary? The reason I hesitate on this is because Ubuntu 14.04 still has an older version of cmake (2.8.12). I would prefer to upgrade after it retires.

@tintou
Copy link
Author

tintou commented Mar 15, 2018

@xuhdev Ah yeah, it's completely possible to not depend on CMake 3.0, I just wanted to use concise variables available since the introduction of the project command. Here is an updated version that doesn't change the CMake requirement.
I have taken your description and just shrunk it a little bit
Regarding the library name, as pkgconfig mostly targets C/C++ it's probably worth being concise here 🤷‍♂️ also I never found mention of the core name (the .so name and headers don't mention it) so it's probably better to keep this like that for discoverability

@xuhdev xuhdev merged commit ab08816 into editorconfig:master Mar 15, 2018
@xuhdev
Copy link
Member

xuhdev commented Mar 15, 2018

Looks good to me. Thanks!

@ffes ffes mentioned this pull request Mar 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xuhdev xuhdev xuhdev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@tintou @xuhdev