Skip to content

Conversation

@asfernandes
Copy link
Member

No description provided.

@AlexPeshkoff
Copy link
Member

Is it good idea to mix our .h files with boost in same directory? May be better add separate dir for our headers.

@asfernandes
Copy link
Member Author

Is it good idea to mix our .h files with boost in same directory? May be better add separate dir for our headers.

Boost is in subdir impl/boost, I don't consider impl files to be together with boost. I would have no problem in create a separate subdir, but note we are already on include/firebird. I would hate to have include/firebird/impl/firebird.

@AlexPeshkoff
Copy link
Member

I've missed the fact that boost is already in separate directory. Forget it.

@AlexPeshkoff
Copy link
Member

Almost OFFT - I suppose distribution packagers will stop forgetting about firebird/impl subdir after this PR ;)

@aafemt
Copy link
Contributor

aafemt commented May 29, 2019

Do plain C include files for old API really belongs to impl dir which AFAIU is supposed to be for implementation of the new API...?

@asfernandes
Copy link
Member Author

Do plain C include files for old API really belongs to impl dir which AFAIU is supposed to be for implementation of the new API...?

Public C include files were ibase.h, ib_util.h and iberror.h. Your supposition of impl being only for the new API is wrong. impl is used for files that should not be included directly by user applications.

@aafemt
Copy link
Contributor

aafemt commented May 29, 2019 via email

@asfernandes
Copy link
Member Author

I'll have no problem to publish more files in include/firebird (of course with better names - no _pub suffix) if others agree. But I think that should be done after this PR, which is about restructure things without new features.

@asfernandes asfernandes merged commit 45d5e3a into master Jun 3, 2019
@asfernandes asfernandes deleted the work/includes branch August 12, 2019 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants