Given the shift to modules, it is not clear that the original include directory structure is the best any more (with IMP/restraints holding restraints, for example). I would propose that all public classes/ functions in the modules API be in headers in the module main include directory. Structuring things this way means that the include paths are the same as the namespace nesting which is the same as the python module structure, which is nice. And the base classes are all together in the include/IMP directory already.
Daniel Russel wrote: > Given the shift to modules, it is not clear that the original include > directory structure is the best any more (with IMP/restraints holding > restraints, for example). I would propose that all public classes/ > functions in the modules API be in headers in the module main include > directory.
Sure, why not? It doesn't really make a whole lot of difference either way - the only reason to have a non-flat directory structure is to make things easier to look at with 'ls' (the structure has no influence on the way modules are linked).
Ben
On Sep 24, 2008, at 9:18 AM, Ben Webb wrote:
> Daniel Russel wrote: >> Given the shift to modules, it is not clear that the original include >> directory structure is the best any more (with IMP/restraints holding >> restraints, for example). I would propose that all public classes/ >> functions in the modules API be in headers in the module main include >> directory. > > Sure, why not? It doesn't really make a whole lot of difference either > way - the only reason to have a non-flat directory structure is to > make > things easier to look at with 'ls' (the structure has no influence on > the way modules are linked). For me, I almost never do ls as opposed to type part of the name and hit tab to list matching names (which is easier with one directory).
I guess the other reason was to avoid conflicts with the SConscript files. But since things can be checked in directly, that shouldn't be a problem any more (and if the files aren't listed on separate lines in the SConscripts, they soon will be, further making conflicts less likely :-)
Daniel Russel wrote: > I guess the other reason was to avoid conflicts with the SConscript > files. But since things can be checked in directly, that shouldn't be > a problem any more (and if the files aren't listed on separate lines > in the SConscripts, they soon will be, further making conflicts less > likely :-)
In a general sense, SVN write access should not be interpreted as an invitation to go in and change everything that you don't like. If you don't like an aspect of my policy, you can bring it up for discussion. And if a policy isn't well defined, we can define it.
Ben
On Sep 24, 2008, at 9:44 AM, Ben Webb wrote:
> Daniel Russel wrote: >> I guess the other reason was to avoid conflicts with the SConscript >> files. But since things can be checked in directly, that shouldn't be >> a problem any more (and if the files aren't listed on separate lines >> in the SConscripts, they soon will be, further making conflicts less >> likely :-) > > In a general sense, SVN write access should not be interpreted as an > invitation to go in and change everything that you don't like. If you > don't like an aspect of my policy, you can bring it up for discussion. > And if a policy isn't well defined, we can define it.
You really don't like to give up control :-) Anyway, the policies aren't yours, they are ours. And, as far as I can tell, it isn't a policy (i.e. it isn't documented as such or wasn't last I looked), it is just the way things happen to be.
As for that particular change the current setup is hard to read, hard to maintain (since you have to either insert things out of alphabetical order or reflow the names), causes SVN conflicts for no reason (since SVN doesn't know when white space is important), hard to automate (it is a bit silly that there is no script to auto generate those files since that would have avoided a number of bugs over the past year) etc... And I have brought it up before in the form of complaints about the files resulting in unnecessary conflicts, replacement files submitted from time to time and scripts to auto generate the files and never was given any reason for the current setup.
As a result, the change does not change any stated policy, has good reasons, and makes some things easier. Seems like exactly the sort of thing that opening up svn is designed to facilitate.
Daniel Russel wrote: > On Sep 24, 2008, at 9:44 AM, Ben Webb wrote: >> In a general sense, SVN write access should not be interpreted as an >> invitation to go in and change everything that you don't like. If you >> don't like an aspect of my policy, you can bring it up for discussion. >> And if a policy isn't well defined, we can define it. > > You really don't like to give up control :-) Anyway, the policies > aren't yours, they are ours. And, as far as I can tell, it isn't a > policy (i.e. it isn't documented as such or wasn't last I looked), it > is just the way things happen to be.
I think you missed my point, which was that (in general) I am wary of blanket statements along the lines of "once I get write access I am going to change everything" which seemed to be what you were saying. That is precisely what others are worried about.
> As for that particular change the current setup is hard to read, hard > to maintain (since you have to either insert things out of > alphabetical order or reflow the names), causes SVN conflicts for no > reason (since SVN doesn't know when white space is important), hard to > automate (it is a bit silly that there is no script to auto generate > those files since that would have avoided a number of bugs over the > past year) etc... And I have brought it up before in the form of > complaints about the files resulting in unnecessary conflicts, > replacement files submitted from time to time and scripts to auto > generate the files and never was given any reason for the current setup.
I was speaking generally, but on this issue specifically it doesn't make a lot of difference to me whether you do the current: files = ['a', 'b', 'c', 'd'] or the proposed: files = [] files.append('a') files.append('b') files.append('c') files.append('d') Actually, in contrast the latter is less efficient Python and looks uglier to me (but the latter is just aesthetic of course). If you really want each file on a separate line, I suggest: files = [ 'a', 'b', 'c', 'd', ] At least that way the Python code is functionally the same as the original, is still easy to auto-generate, and isn't as unnecessarily verbose.
The only bugs I can think of were header files not being installed. That is no longer an issue because they are staged to the build directory now, so the compile (not just the install) will fail if a file is missed.
Ben
On Sep 26, 2008, at 9:16 AM, Ben Webb wrote:
> Daniel Russel wrote: >> On Sep 24, 2008, at 9:44 AM, Ben Webb wrote: >> You really don't like to give up control :-) Anyway, the policies >> aren't yours, they are ours. And, as far as I can tell, it isn't a >> policy (i.e. it isn't documented as such or wasn't last I looked), it >> is just the way things happen to be. > > I think you missed my point, which was that (in general) I am wary of > blanket statements along the lines of "once I get write access I am > going to change everything" which seemed to be what you were saying. > That is precisely what others are worried about. "Changing everything" would take a lot of time :-) I said I would clean up the formatting of those files (and, I didn't say it, but would like to provide a script to autogenerate them since they provide no extra information about IMP beyond the directory structure).
>> > I was speaking generally, but on this issue specifically it doesn't > make > a lot of difference to me whether you do the current: > files = ['a', 'b', 'c', 'd'] > or the proposed: > files = [] > files.append('a') > files.append('b') > files.append('c') > files.append('d') > Actually, in contrast the latter is less efficient Python and looks > uglier to me (but the latter is just aesthetic of course). If you > really > want each file on a separate line, I suggest: > files = [ > 'a', > 'b', > 'c', > 'd', > ] > At least that way the Python code is functionally the same as the > original, is still easy to auto-generate, and isn't as unnecessarily > verbose. Actually I was proposing the last way since it has less text. An earlier version of my script had done the middle way, which as you point out is a bit ugly and hard to read. I can't imagine the efficiency makes any difference unless python is way worse than I imagine.
> The only bugs I can think of were header files not being installed. > That > is no longer an issue because they are staged to the build directory > now, so the compile (not just the install) will fail if a file is > missed. Yes, there were several instances of headers not being installed. I like that the failure occurs earlier now, but I still don't see any reason to make people maintain those lists by hand. And having the files on separate lines does reduce the number of conflicts and makes diffs clearer (I have spent too much time sorting out conflicts in the SConscript files--this is part of the reason I switched to a script which just rewrites them all).
participants (2)
-
Ben Webb -
Daniel Russel