On 09/30/2015 07:42 AM, Lukas Slebodnik wrote:
On (29/09/15 17:45), Jakub Hrozek wrote:
On Tue, Sep 29, 2015 at 11:38:58AM +0200, Sumit Bose wrote:
On Tue, Sep 29, 2015 at 10:50:06AM +0200, Pavel Reichl wrote:
On 09/29/2015 10:44 AM, Lukas Slebodnik wrote:
On (29/09/15 10:42), Pavel Reichl wrote:
On 09/29/2015 10:35 AM, Lukas Slebodnik wrote:
On (29/09/15 10:16), Pavel Reichl wrote:
On 09/29/2015 10:08 AM, Lukas Slebodnik wrote:
On (29/09/15 08:45), Pavel Reichl wrote:
On 09/29/2015 08:31 AM, Lukas Slebodnik wrote:
On (27/09/15 12:49), Pavel Reichl wrote:
Hello, please see trivial patch attached.
>From b9f938087973444f0ec26fc24ad68dca7ac63034 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <reichl.pa...@gmail.com>
Date: Sun, 27 Sep 2015 12:34:20 +0200
Subject: [PATCH] confdb: Remove unused function confdb_get_long
---
src/confdb/confdb.c | 51 ---------------------------------------------------
1 file changed, 51 deletions(-)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index
c097aad7745eda4fff051c7da027776f95db0f03..eebd478f74041d2050df9edc283df43a65462340
100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -475,57 +475,6 @@ failed:
return ret;
}
-long confdb_get_long(struct confdb_ctx *cdb,
- const char *section, const char *attribute,
- long defval, long *result)
-{
Would it be better to consider this function as confdb API
and add to src/confdb/confdb.h?
It could be.
It might be useful in the future.
I thought that out policy towards unused functions were to remove them.
Could you point me to the description of such policy?
I'm not aware of it.
I don't think it's written anywhere
Good to know. I thought I missed something
I just saw we did it repeatedly before and thought it's our general practice.
One more time:
If we consider confdb as library than we should never remove functions.
Removing functions from other parts of code is something
else.
We did so in commit 2b94ab415b30861f42b68725d9231905baf8c3bd
I didn't notice that patch. I'm sorry I do not have a time
to follow each patchset.
We can always resurrect removed code if needed.
If we consider confdb as library than
we should never remove functions.
Then we should decide which parts of SSSD are public libraries and which are
not, then we could avoid this kind of discussions.
I wrote "consider confdb as library" and not as a public library with
stable API. And libraries many times contains function which are not used.
So we should not remove them. Even though it would be still in git history.
Why? If we don't use them and nobody else does why do we have to keep them?
libraries many times contains function which are not used
Well, but if they are not public we can change that, right? We can change them
as we need and removing unused code is one of those things. I don't see the
difference between private library and 'other parts of code'.
I think we should be explicit here. If we want to consider confdb as a
library we should make it a public library with a stable API with all
the consequences. In general I think this is not a bad idea e.g. with
respect to all the config related tickets we have planned for 1.14. But
we should define the initial version of a stable API, based on the
result and requirements from the 1.14 tickets.
I'm more with Simo here that we shouldn't consider confdb and sysdb as
stable ABIs, at least not yet. I think we need the flexibility of a private,
unstable library.
At the same time, I think we should try harder to avoid polluting the
sysdb/confdb APIs with new calls and in general strive to make even the
internal APIs easily usable. Here I agree with Lukas that the
current interface is not very nice -- and the sysdb interface would be
something that any new external contributor would be confronted with.
Maybe when we review (hint, nudge, nudge) Michal's sysdb patches, we can
take a look at the API and massage it a bit, move related functions
together in the header files and merge or remove extra functions? (Yes,
we can also file a ticket, but I don't think it would land in a
milestone where we would touch it soon...)
Having private/internal libraries with a more relaxed policy might turn
out to be problematic as e.g. can be seen by some internal samba
libraries which are used by us, OpenChange and maybe other projects
where upstream changes might force changes in the other projects as
well. Ok, the argument here is that they shouldn't have been used in the
first place, but what's the point in re-implementing existing
functionality.
Currently I wouldn't see libsss_util where confdb.c is included as a
library at all, but just as a mean to make the build process easier and
faster.
Coming to the original topic of the discussion, I would agree to remove
confdb_get_long() because it is nowhere used (and it looks like it
never was) and it does not even have a test.
Is that an ack? :-)
No,
I think this was rather question for Sumit AFAIK he was the author of quoted
text.
I still do not think that removing function and adding it back after few months
The problem is - why do you think we will need the function in future? Using
this logic we could never remove any function from private libs.
is a good idea. Better would be to include to confdb.h and write tests.
But it seems that I'm a single person in opposition.
So someone else need to ACK it.
LS
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
