To paraphrase, I'm extremely negative on combining non-API related utility headers with TS API headers. They're really quite different and that's something users (such as me) writing plugins would care about. I would greatly appreciate them being distinct. Separating API from utility mechanisms is quite a common pattern in open source as well. Beyond that, I still think I'd like to have clear indicator about whether I have to put extra link information in my makefile - linking to libtscpp doesn't happen automatically and knowing that I need to do it is not a trivial point when building a plugin.
I'll update the PR with the CPP API library rename after lunch. On Thu, May 24, 2018 at 11:01 AM, Bryan Call <bc...@apache.org> wrote: > It would be best to combine include/tscpp and include/tsutil into one > directory (include/tscpp). External users writing plugins really don’t > care about the dependency distinction between them. > > This would mean that would would need to move the files from the lib/ts > directory over to the lib/cppapi directory. Also, the cppapi library should > be called libtscpp.so to be in sync with the include directory name. > > -Bryan > > > > > On May 24, 2018, at 6:45 AM, Alan Carroll <solidwallofc...@oath.com.INVALID> > wrote: > > > > Based on feedback here, on IRC, and on the PR, the changes have been > > updated. > > > > Exported headers have been moved to the top level 'include' directory. > > > > include/ts -> C API headers. > > include/tscpp -> C++ API headers. > > include/tsutil -> C++ utility headers. > > > > The same directories are used for the install. This reduces differences > > between in core plugins and external plugins - both will now use the same > > include paths for the same headers. > > > > Source changes - > > > > lib/ts -> lib/tsutil : this removes the problem where 'ts' as part of the > > include path meant different things externally vs. internally. Now 'ts' > is > > always the C API headers and 'tsutil' are the utility headers. This also > > means 'libtsutil.so' is built in 'lib/tsutil' which is nice. As part of > > this 'apidefs.h.in' and 'apidefs.h' were moved to 'include/ts' because > > those are C API headers. The include path for this file remains > > 'ts/apidefs.h'. > > > > On Wed, May 23, 2018 at 4:31 PM, Jason Kenny <jke...@oath.com.invalid> > > wrote: > > > >> I gave a talk on this my first summate. Honestly, most people I talk to > are > >> +1 on this. > >> > >> The only complaint about fixing our layout and refactoring broken code > was > >> that it makes it hard to cherry pick back to older drops. People > complain > >> and hack the build more and more. I want to: > >> > >> 1) simplify the build > >> 2) make it easier for people to find the real source. > >> 3) make it easier to change or add api without coming up with odd and > >> overly clever names or tricks to avoid having a clean layout > >> > >> I believe in the end cleaner code will only make it easier for people to > >> want to help out and join the ATS community. making complex as it scares > >> people away to other projects > >> > >> Jason > >> > >> On Wed, May 23, 2018 at 4:25 PM, Leif Hedstrom <zw...@apache.org> > wrote: > >> > >>> > >>> > >>>> On May 23, 2018, at 3:20 PM, Jason Kenny <jke...@oath.com.INVALID> > >>> wrote: > >>>> > >>>>>> 1) I don’t like the “src” top level directory, that implies that all > >>>> source is there (which is clearly not the case) > >>>> We cannot move everything at once. We have to do stuff in small steps. > >>>> > >>>> I would really like to get all the source under an src/ dir to solve a > >>>> number of stupid build problems we have. We have to start someplace, > >> and > >>>> honestly this is a good first step. > >>> > >>> Hugely -1 on this ninja move. > >>> > >>> This should be discussed / suggested in a separate discussion / PR, and > >>> not snuck in like this into another PR. I.e. do not do this here, but > if > >> we > >>> decide to change this later, make that change there. > >>> > >>> I’d be curious to hear about your build problems too, in the last 10+ > >>> years, no one has had any such issues because of the lack of a src/, > >> that I > >>> know of at least. > >>> > >>> Ciao, > >>> > >>> — leif > >>> > >>> > >> > >
