-
Notifications
You must be signed in to change notification settings - Fork 934
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.
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
[libut]support shared build and more #84
base: master
Are you sure you want to change the base?
Conversation
all: $(OBJS) | ||
SHELL=/usr/sh | ||
VERSION=2.0.1 | ||
MAJOR=2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'd prefer to see these variables inlined into the places where they're currently being used, to keep the code as straightforward as possible. I infer from context that MAJOR
is always supposed to be the major version, e.g. if VERSION
became 3.0.0
then MAJOR
should become 3
, but I don't really understand why. This would probably be easier to understand if half of the logic weren't located 30 lines away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean...to use "2.0.1" directly instead of $(VERSION)? then 2.0.1 and 2 will become magic numbers, you'll have to change all of them in this Makefile at version updates. Or you just want to move them down to the place where they are about to be used right away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, make them magic numbers. Ideally, when it's time to bump the version, I want to be able to git grep 2.0.1
and find exactly all the places that need adjustment; and when it's not time to bump the version, I want the code to be simple and straightforward, with as few named entities and as little cognitive overhead as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok.
Hi, I've reseted my branch and made everything desirable. If you can make a final decision on Marguerite |
Well, I've made these changes as well in my current Fedora-specfile. The only thing, which I can see on your changes, that is a bad idea, is the fact you are using the "regular" version of uthash for the so-versioning. According to the manuals to linux/gnu LD, the so-versioning works differently. libut.so.2.0.1 means in versioning terms of so-libs, that it is recently version 2, the latest compatibility is so-version 0 (which makes it incompatible with so-version 1) and the earliest compatibility version is so-version 1 (contradiction to so-version 0). See Libtool's versioning scheme for further details on how so-libs are versioned correctly. |
FWIW, my opinions on the subject are still
|
@besser82 Thanks! I'll change it to 2.0.1 as yours. |
These commits were tested on openSUSE.