Yes, let's go for a global change for the header ordering, it's out of scope for this pull request. However, I'd argue that we should put our headers first, to ensure that we are not missing any includes (e.g. using std::cout without including <iostream> in the header, but getting it only as side effect of previous includes). Also, if we use "" instead of <> when including, then I think we need to leave the base directory out. So, for example, we do either #include <XrdHttp/XrdHttpReq.hh> or #include "XrdHttpReq.hh" in src/XrdHttp/XrdHttpReq.cc, but we will handle this later in another pull request.

Changing the ordering would involve a massive number of files. I don't think he "strd" problem is sufficient to warrant a massive change like that, especially since we've been doing it this way for 20+ years. Yes, I know there are good reasons to do it the other way because then you are forced to include every dependency all the time for system headers. Though you still have the same problem for local headers. So, in the end to avoid busy work, I'd do the minimum change and simply correct the files in http to follow what is done elsewhere.

As for putting on the directory ahead of an include file, again this is stylistic. We don't need to worry about this as long as people follow the naming convention for files as it all works out in the end. Requiring "src/" prefix is silly and it not needed. It works fine without that. The cmake include spec makes things work in a natural way. So, I'm neutral but emphasize there is no reason to change everything that has a directory prefix to not have one or add "src/" everywhere. That's a nonsense change IMO.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: <xrootd/xrootd/pull/1950/c1480409966@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/1950#issuecomment-1480409966", "url": "https://github.com/xrootd/xrootd/pull/1950#issuecomment-1480409966", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1