Coder Social home page Coder Social logo

Allow usage as sub-project about libzippp HOT 9 CLOSED

ctabin avatar ctabin commented on July 3, 2024
Allow usage as sub-project

from libzippp.

Comments (9)

ctabin avatar ctabin commented on July 3, 2024

@Flamefire Thanks for your report. Could you submit a PR with the needed changes ? That would be great !

from libzippp.

Flamefire avatar Flamefire commented on July 3, 2024

I will. What about my second point? Needs an owner decision ;)

from libzippp.

ctabin avatar ctabin commented on July 3, 2024

@Flamefire Thanks for your feedback. It makes sense to me to have a subfolder. Maybe we can make that configurable somehow ?

from libzippp.

Flamefire avatar Flamefire commented on July 3, 2024

While I understand the idea of having it configurable I'd rather not. Every option added must/should be tested so every option added increases the amount to test by a factor of 2. (On/off with every other option)

Downside of the subfolder: currently installed project uses no subfolder in the include, but a subfolder in the install: installed to include/libzippp/libzippp.h but #include <libzippp.h> due to the subfolder being added to -I, so that would be a breaking change. However as it is probably installed into some global location for users not using CMake they could be using #include <libzippp/libzippp.h> without using any -I already.

So IMO: Use a subfolder, bump the major version as per SemVer, don't use an option toggle. Oh and while doing that: Maybe require C++11? Should be supported wide enough already and allows to replace the typedefs at

libzippp/src/libzippp.h

Lines 55 to 70 in be75a34

#ifdef WIN32
typedef long long libzippp_int64;
typedef unsigned long long libzippp_uint64;
typedef unsigned short libzippp_uint16;
//special declarations for windows to use libzippp from a DLL
#define LIBZIPPP_SHARED_LIBRARY_EXPORT __declspec(dllexport)
#define LIBZIPPP_SHARED_LIBRARY_IMPORT
#else
//standard ISO c++ does not support long long
typedef long int libzippp_int64;
typedef unsigned long int libzippp_uint64;
typedef unsigned short libzippp_uint16;
#define LIBZIPPP_SHARED_LIBRARY_EXPORT
#define LIBZIPPP_SHARED_LIBRARY_IMPORT
#endif
by their standard equivalents as well as e.g. returning a unique_ptr from fromBuffer and/or making it movable. I also noticed that your MakeFile already adds -std=c++0x although it doesn't seem to be used. Finally: Maybe drop the makefile completely? It is not a "clean" Makefile to be used e.g. by package maintainers, so rather factor out the dependency installation into shell scripts (code like in my updated travis.yml) and announce CMake as the "official" way.
Just mentioning all this so it can be considered when bumping the major version.

from libzippp.

Flamefire avatar Flamefire commented on July 3, 2024

@ctabin Take a look at my PR #52 which implements the basics and puts CMake into focus. The include path change is not done as I'm waiting for your decisions. Besides that I think the setup and testing is pretty solid.

from libzippp.

ctabin avatar ctabin commented on July 3, 2024

@Flamefire That's very promising ! Regarding the include, the goal is to follow the standards as much as possible, and also to keep it simple. I wonder how vcpkg will behave, since libzippp has been included in it (see microsoft/vcpkg#6801 and #46).

Since this will change the building of libzippp, I don't mind making a new major version with some breaking changes, such as the include, so let's go 👍

from libzippp.

Flamefire avatar Flamefire commented on July 3, 2024

I wonder how vcpkg will behave, since libzippp has been included in it (see microsoft/vcpkg#6801 and #46).

That is indeed an issue. Users will use the targets and expect #include "libzippp.h" to work. Maybe the better solution would be to skip the subfolder and install libzippp.h into include directly assuming this will always be the only header file? This would unify usage with non-cmake there libzippp is installed into /usr/local and hence #include "libzippp.h would work without any -I. However this would break such uses of the current libzippp where #include "libzippp/libzippp.h" is already used without any -I.

Maybe: Use the subfolder, but add include AND include/libzippp to the include path of the targets and announce one of them to be deprecated/discouraged so users should start using the other.

Then which one? If there will only be 1 header then #include "libzippp/libzippp.h" looks rather ugly :/

Maybe just keep it as-is. IMO there is no real standard for handling stuff like that, so...

from libzippp.

ctabin avatar ctabin commented on July 3, 2024

@Flamefire I don't see any other header than libzippp.h in a near future. Actually, this library is intended to be small, since it is only a wrapper.

I would say to keep things as-is, having #include "libzippp/libzippp.h does'nt look so ugly to me 😅

from libzippp.

Flamefire avatar Flamefire commented on July 3, 2024

Ok then it will stay as #include "libzippp.h" for CMake users and #include "libzippp/libzippp.h# for users using default install location and no -I or CMake

from libzippp.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.