{"action":"created","issue":{"url":"https://api.github.com/repos/dropbox/djinni/issues/247","repository_url":"https://api.github.com/repos/dropbox/djinni","labels_url":"https://api.github.com/repos/dropbox/djinni/issues/247/labels{/name}","comments_url":"https://api.github.com/repos/dropbox/djinni/issues/247/comments","events_url":"https://api.github.com/repos/dropbox/djinni/issues/247/events","html_url":"https://github.com/dropbox/djinni/pull/247","id":163698303,"number":247,"title":"Move public headers to include/djinni","user":{"login":"mknejp","id":5156768,"avatar_url":"https://avatars.githubusercontent.com/u/5156768?v=3","gravatar_id":"","url":"https://api.github.com/users/mknejp","html_url":"https://github.com/mknejp","followers_url":"https://api.github.com/users/mknejp/followers","following_url":"https://api.github.com/users/mknejp/following{/other_user}","gists_url":"https://api.github.com/users/mknejp/gists{/gist_id}","starred_url":"https://api.github.com/users/mknejp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mknejp/subscriptions","organizations_url":"https://api.github.com/users/mknejp/orgs","repos_url":"https://api.github.com/users/mknejp/repos","events_url":"https://api.github.com/users/mknejp/events{/privacy}","received_events_url":"https://api.github.com/users/mknejp/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/dropbox/djinni/labels/enhancement","name":"enhancement","color":"84b6eb"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2016-07-04T15:27:38Z","updated_at":"2016-07-27T02:47:34Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/dropbox/djinni/pulls/247","html_url":"https://github.com/dropbox/djinni/pull/247","diff_url":"https://github.com/dropbox/djinni/pull/247.diff","patch_url":"https://github.com/dropbox/djinni/pull/247.patch"},"body":"This separation of public and private headers allows users to specify `include/` as obvious header search path and then use `#include <djinni/...>` as reliable include prefix. This also gives a sensible default for all the `--xxx-base-lib-include-prefix` values.\r\n\r\nNote this is a breaking change but I think it is important to have a clear separation of public and private files as discussed in the mobilecpp group. It also makes it more obvious from the folder structure what users have to put as their header search path when using djinni as well as to a more \"standardized\" and unambiguous way of specifying support library include prefixes when generating files."},"comment":{"url":"https://api.github.com/repos/dropbox/djinni/issues/comments/235466826","html_url":"https://github.com/dropbox/djinni/pull/247#issuecomment-235466826","issue_url":"https://api.github.com/repos/dropbox/djinni/issues/247","id":235466826,"user":{"login":"mknejp","id":5156768,"avatar_url":"https://avatars.githubusercontent.com/u/5156768?v=3","gravatar_id":"","url":"https://api.github.com/users/mknejp","html_url":"https://github.com/mknejp","followers_url":"https://api.github.com/users/mknejp/followers","following_url":"https://api.github.com/users/mknejp/following{/other_user}","gists_url":"https://api.github.com/users/mknejp/gists{/gist_id}","starred_url":"https://api.github.com/users/mknejp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mknejp/subscriptions","organizations_url":"https://api.github.com/users/mknejp/orgs","repos_url":"https://api.github.com/users/mknejp/repos","events_url":"https://api.github.com/users/mknejp/events{/privacy}","received_events_url":"https://api.github.com/users/mknejp/received_events","type":"User","site_admin":false},"created_at":"2016-07-27T02:47:33Z","updated_at":"2016-07-27T02:47:33Z","body":"> My general strategy for avoiding imposing new requirements or compatibility breakages on Djinni's users has been to make everything optional/configurable. The large number of command-line arguments are the result of that.\r\n\r\nThat's certainly a noble goal but at some point one has to ask themselves whether supporting/maintaining a bazillion options is worth the trouble (especially if people end up not using these options).\r\n\r\n> I find \"src\" less obviously private on its own, but when put next to include it's pretty clear, and would be even moreso if someday in a theoretical install there were also a \"libs\" directory for pre-built binaries.\r\n\r\nCMake's `install` target in #248 does exactly that as it's using the default `GNUInstallDirs` script. And the point about include prefixes is even more important there because all targets are installed to the same location. So you end up with folders\r\n`install/bin/*`\r\n`install/include/*`\r\n`install/lib/*`\r\nThe intention is for users to set `install/include/` as their single search path and get all the things at once. And at that point you really don't want to dump all the headers of all targets into `install/include/` without subfolders as disambiguation prefix. Just imagine how many libraries have a file called `config.h`...\r\n\r\nAnother issue that just crossed my mind however is the include prefix for Apple frameworks. #248 currently offers the ability to build a `.framework` and the canonical Apple way of doing things is to have the includes in a `.framework` be prefixed with the framework name, meaning if you have `foo.framework` then Xcode/Clang modify the includes automatically in a way that `#include <foo/x.h>` works as expected (and actually refers to `foo.framework/Headers/x.h`), otherwise one has to do manual project patching. That also applies to the headers inside the framework referencing each other.\r\n\r\nSpecifically in the case of Djinni we end up with paths like `#include <djinni/objc/DJIMarshal.h>` or `#include <djinni/jni/Marshal.hpp>` when using `djinni.framework` (which is what could be distributed as a prebuilt binary at some point).\r\n\r\nSo the question remaining here is: does the compromise include having `djinni` as common prefix for public headers?"}}
{"actor":{"display_login":"mknejp"}}