Thread overview | |||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
After having updated there are reams of code that doesn't work. There are a few issues that I think we need to be careful of: * There are absolutely huge wads of code versioned under D_ddoc, that has rotten and prevents documentation generation. This means code was committed without compiling. Please compile all code with the relevant flags before committing. * We already have version=ddoc in posix.mak for documentation. Is D_Ddoc predefined somewhere? In * Walter and I have very different styles but generally are trying to make every line of Phobos count. I have no doubt examples can be found to the contrary, but please let's stop this trend of verboseness like: @property string name() const { assert(0, "No implementation. Function exists only for DDoc generation"); } Just don't define the function! There are many examples. Whenever inserting a line of code, please ask yourself what amount of work that line does. I'll try to fix things now. Thanks, Andrei |
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Fri, 21 Jan 2011, Andrei Alexandrescu wrote:
> After having updated there are reams of code that doesn't work. There are a few issues that I think we need to be careful of:
>
> * There are absolutely huge wads of code versioned under D_ddoc, that has rotten and prevents documentation generation. This means code was committed without compiling. Please compile all code with the relevant flags before committing.
What needs to be added to the auto-tester to catch this near submit time? Is it another make target? If so, what?
Is it platform specific (ie, should only be run on win32)? If so, which?
Should it be added to the unittest target (probably not, just asking)?
Is there anything _other_ than doc generation that's broken right now? If so, what?
<snip not-broken style issues -- not that I disagree, just partitioning the discussion>.
Thanks,
Brad
|
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Friday, January 21, 2011 18:42:16 Andrei Alexandrescu wrote:
> After having updated there are reams of code that doesn't work. There are a few issues that I think we need to be careful of:
>
> * There are absolutely huge wads of code versioned under D_ddoc, that has rotten and prevents documentation generation. This means code was committed without compiling. Please compile all code with the relevant flags before committing.
>
> * We already have version=ddoc in posix.mak for documentation. Is D_Ddoc predefined somewhere? In
>
> * Walter and I have very different styles but generally are trying to make every line of Phobos count. I have no doubt examples can be found to the contrary, but please let's stop this trend of verboseness like:
>
> @property string name() const
> {
> assert(0, "No implementation. Function exists only for DDoc
> generation"); }
>
> Just don't define the function! There are many examples. Whenever inserting a line of code, please ask yourself what amount of work that line does.
>
> I'll try to fix things now.
Well, I know that I'm guilty of that line. I didn't know that you _could_ declare a function with no definition. I would have done that had I known that it was legal.
There does seem to be a tendency in Phobos to put ddoc comments on the Windows versions of functions if there are multiple versions, which means that building ddoc on posix isn't going to look anything like building it on Windows. I'd suggest that if a function has to be versioned for different OSes that either the versioning be done in the function body or that the function be put in version(D_Ddoc) with the documentation only on that version (though obviously without a body for the function, unlike what I've been doing).
And thanks for pointing out how to build the documentation on posix. I didn't know that you could. I'll certainly make sure to do that before checking anything in in the future.
- Jonathan M Davis
|
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jonathan M Davis | On 1/21/11 8:59 PM, Jonathan M Davis wrote: > Well, I know that I'm guilty of that line. I didn't know that you _could_ declare a function with no definition. I would have done that had I known that it was legal. Okay, sorry for snapping. As I guess it's easy to figure, it was out of passing frustration. I'll remove the bodies now. (Wow, I feel like a mobster.) > There does seem to be a tendency in Phobos to put ddoc comments on the Windows versions of functions if there are multiple versions, which means that building ddoc on posix isn't going to look anything like building it on Windows. I'd suggest that if a function has to be versioned for different OSes that either the versioning be done in the function body or that the function be put in version(D_Ddoc) with the documentation only on that version (though obviously without a body for the function, unlike what I've been doing). Yes, I think this is a good idea. But... yellow? :o) Oh, one more thing. Try to use $(MACRO arg, arg) instead of HTML-specific tags. For example, to insert an anchor just say $(WEB foo.com, bar). Give me a few minutes and I'll checkin. Shoo, if you could look into the Windows issue, that would be great. Thanks! Andrei |
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | On 1/21/11 8:56 PM, Brad Roberts wrote: > What needs to be added to the auto-tester to catch this near submit time? Is it another make target? If so, what? It's make -f posix.mak html that fails on all systems. > Is it platform specific (ie, should only be run on win32)? If so, which? On win32 it can't be run as Windows is not quite posix :o). > Should it be added to the unittest target (probably not, just asking)? I think it's a good idea! > Is there anything _other_ than doc generation that's broken right now? If so, what? Apart from what Shoo found, I don't know of any. > <snip not-broken style issues -- not that I disagree, just partitioning the discussion>. I'd love to have a minimal stylistic checker upon checkin. For one thing, I don't find any reason for two empty lines and I don't plan to rearrange the monitor sideways for more vertical space. Andrei |
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Friday, January 21, 2011 19:33:24 Andrei Alexandrescu wrote: > On 1/21/11 8:59 PM, Jonathan M Davis wrote: > > Well, I know that I'm guilty of that line. I didn't know that you _could_ declare a function with no definition. I would have done that had I known that it was legal. > > Okay, sorry for snapping. As I guess it's easy to figure, it was out of passing frustration. > > I'll remove the bodies now. (Wow, I feel like a mobster.) LOL. I think that it's totally understandable that you weren't happy with the situation. I've never been able to figure out how to generate the docs in Linux (too bad at reading makefiles I guess - that and the impression from the documentation in Phobos itself gave me the impression that we weren't even trying to build the docs on Linux), and since you don't separate declarations and definitions in D, it never occurred to me that you could have functions with no bodies. So, apparently, I broke some stuff and didn't catch it. My apologies. > > There does seem to be a tendency in Phobos to put ddoc comments on the Windows versions of functions if there are multiple versions, which means that building ddoc on posix isn't going to look anything like building it on Windows. I'd suggest that if a function has to be versioned for different OSes that either the versioning be done in the function body or that the function be put in version(D_Ddoc) with the documentation only on that version (though obviously without a body for the function, unlike what I've been doing). > > Yes, I think this is a good idea. But... yellow? :o) I'm fine with a different color. Given that I didn't know how to generate the Phodos docs, I haven't even seen how that looks yet. Green and red both seemed wrong since green implies "go" or "good" whereas red is obviously for problems or warnings. So, I went with yellow. I'm not married to that color choice by any means. I just thought that it was a good idea to mark functions as Posix or Windows-only and put that note in a color other than black so that it would get noticed. > Oh, one more thing. Try to use $(MACRO arg, arg) instead of > HTML-specific tags. For example, to insert an anchor just say $(WEB > foo.com, bar). I think that I haven't been doing that because the URLs never actually appeared in the documentation when I did that. Perhaps it's different when generating with the Phobos makefiles though (even though I _was_ using the same .css and ddoc style files as Phobos...). > Give me a few minutes and I'll checkin. Shoo, if you could look into the Windows issue, that would be great. Thanks! I'll look over std.datetime (and std.file since I recently messed with that pretty thoroughly as far documentation goes) to make sure that the obvious stuff is fixed, though I'd assume that you fixed at least std.file, since that's where SHOO's problem was. I'm going to be redoing some of std.datetime's documentation soon anyway, since I think that I duplicated a bit too much across various template specializations, and I generally put ddoc comments on _every_ version of a function rather than just version(D_Ddoc). So, hopefully any unnecessary documentation in std.datetime should be removed soon (though I'd generally rather err on the side of over-documenting than under-documenting). - Jonathan M Davis |
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Fri, 21 Jan 2011, Andrei Alexandrescu wrote: > On 1/21/11 8:56 PM, Brad Roberts wrote: > > What needs to be added to the auto-tester to catch this near submit time? Is it another make target? If so, what? > > It's make -f posix.mak html that fails on all systems. Gah.. what the hell does it need wine for? How twisted is that? It only runs via the posix.mak file, but runs the windows binaries? Please god, why? > > Is it platform specific (ie, should only be run on win32)? If so, which? > > On win32 it can't be run as Windows is not quite posix :o). At one point, the docs were only generated on windows, or at least that's my memory of Walter's process. > > Should it be added to the unittest target (probably not, just asking)? > > I think it's a good idea! Get it to not require wine and the windows version of the compiler and I'll be happy to. > > Is there anything _other_ than doc generation that's broken right now? If so, what? > > Apart from what Shoo found, I don't know of any. Ok, just checking.. you started it off with a rather broader rant about the state of things. Later, Brad |
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu |
Andrei Alexandrescu wrote:
>
> * We already have version=ddoc in posix.mak for documentation. Is D_Ddoc predefined somewhere?
The -D flag predefines it.
|
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | On 1/21/11 10:25 PM, Brad Roberts wrote: > On Fri, 21 Jan 2011, Andrei Alexandrescu wrote: > >> On 1/21/11 8:56 PM, Brad Roberts wrote: >>> What needs to be added to the auto-tester to catch this near submit time? Is it another make target? If so, what? >> >> It's make -f posix.mak html that fails on all systems. > > Gah.. what the hell does it need wine for? How twisted is that? It only runs via the posix.mak file, but runs the windows binaries? Please god, why? Sorry, I meant "OSX and Linux" instead of "all systems". Wine is present for two reasons: 1. Allows people who don't use Windows to test on a Windows-like platform 2. Is a good check for the portability of our Windows code across different Windows versions. But it's not required. Feel free to ignore it. Unless you say OS=win32wine while building it won't bother you. >>> Is it platform specific (ie, should only be run on win32)? If so, which? >> >> On win32 it can't be run as Windows is not quite posix :o). > > At one point, the docs were only generated on windows, or at least that's my memory of Walter's process. Right. I think Jonathan improved the state of affairs because he made documentation available for both OSs. >>> Should it be added to the unittest target (probably not, just asking)? >> >> I think it's a good idea! > > Get it to not require wine and the windows version of the compiler and I'll be happy to. Shouldn't be necessary. Let me know if you hit a hitch. Andrei |
January 21, 2011 [phobos] lots of spurious code that doesn't work | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Friday 21 January 2011 19:48:09 Andrei Alexandrescu wrote:
> I'd love to have a minimal stylistic checker upon checkin. For one thing, I don't find any reason for two empty lines and I don't plan to rearrange the monitor sideways for more vertical space.
LOL. std.datetime would fail miserably on that one. I don't think that I have two lines empty lines within any functions, but pretty much all of the functions, structs, and classes have two empty lines between them rather than one. Single empty lines are used within functions and between a function and its corresponding unittest block, but aside from that most everywhere in std.datetime has two empty lines. Particularly with documentation at the beginning of every function, it seemed easier to read that way. It's pretty rare that I'd put two empty lines in a function though. That would just be wasted space. I tend to use it to separate distinct entities rather than spacing within an entity.
- Jonathan M Davis
|
Copyright © 1999-2021 by the D Language Foundation