On 7/28/06, Matthew Jenkins <mattj@inty.net> wrote:

I have just taken a look at the links module, and it seem to do quite a bit more than the linksdb module.

I do recall looking at it once and thinking 'what on earth is all this? does it do what I want? it looks far too complex. An API? just for links? eek!' so I made LinksDB which is small, simple, and does just what I (and, judging by the feedback I've had from it, many other people) want.

Links not being actively driven forward and actually completed has made "competing" solutions appear. It's unfortunate, but it happens. There is also Easylinks: http://drupal.org/project/easylinks Also, CCKs URL field plus Views lets people build flexible links directories. -- Boris Mann Vancouver 778-896-2747 San Francisco 415-367-3595 Skype borismann http://www.bryght.com

On 28-Jul-06, at 9:05 AM, Jeff Eaton wrote:

I'm not sure that's an accurate characterization. SysCrusher hasn't had a chance to work on the administrative screen for bulk link maint. work, but the module has been good to go under 4.7 for months now. It has a robust API, very solid views integration, and is easy to integrate with. The biggest problem is that 1) its description has an outdated warning, and 2) it hasn't been officially branched. That latter problem is definitely a big one, but I'd really suggest anyone considering implementing a links management system consider links.module and links.inc as their starting point.

Obviously, some would prefer a smaller focused all in one solution. :) No problem with that. But the links package itself is definitely not in disrepair.

Would neglected be a better word? 1 & 2 as you mention above are indicators to me that not enough attention is being paid to the bundle (I'm not trying to diss links, just trying to find out more about future direction / viability). And yes, the other big thing is the does-it-all nature... -- Boris

Funny that this should come up now. I was just talking about it on IRC last night. As a user wanting to implement links on my site, I wanted to know what advantage using linksDB has over CCK & views and was pointed at the one in CVS as well. So now I have three things to choose from and am still not sure. Having multiple ways to do things can be nice but also daunting when you are trying to figure out what to implement. If there is an existing module for some functionality and someone writes a similar module, it would be lovely if they would take a few minutes to put in the module description what theirs does different, why you would want to use one of the other, stuff like that. It doesn't have to be a fancy comparison chart or anything, but just a quick paragraph would do wonders to ease the user confusion. MichelleC ----- Original Message ----- From: "Jeff Eaton" <jeff@viapositiva.net> To: <development@drupal.org> Sent: Friday, July 28, 2006 1:28 PM Subject: RE: [development] LinksDB vs. Links

Possibly; I would be perfectly happy to see the incomplete administration screen just go away and call the module 'released'. It's a perfectly viable module and API package without that one screen, and just as stable as image.module, say. Branching is more of an issue. SysCrusher has continued to work on it, commit patches, and participate in issue queues over the past several weeks so I think it's more a question of calling the module 'done' and 'ready,' which it wasn't before 4.7 shipped.

I suppose it's just frustrating to see people look at an existing module that does 95% of what's needed, and decide to reimplement from scratch to get the extra 5%. There are more and more cases of it every day, it seems. Sometimes it's unavoidable but often it's a real waste.

--Jeff

-----Original Message----- From: Boris Mann [mailto:boris@bryght.com] Sent: Friday, July 28, 2006 1:22 PM To: development@drupal.org Subject: Re: [development] LinksDB vs. Links

On 28-Jul-06, at 9:05 AM, Jeff Eaton wrote:

I'm not sure that's an accurate characterization. SysCrusher hasn't had a chance to work on the administrative screen for bulk link maint. work, but the module has been good to go under 4.7 for months now. It has a robust API, very solid views integration, and is easy to integrate with. The biggest problem is that 1) its description has an outdated warning, and 2) it hasn't been officially branched. That latter problem is definitely a big one, but I'd really suggest anyone considering implementing a links management system consider links.module and links.inc as their starting point.

Obviously, some would prefer a smaller focused all in one solution. :) No problem with that. But the links package itself is definitely not in disrepair.

Would neglected be a better word? 1 & 2 as you mention above are indicators to me that not enough attention is being paid to the bundle (I'm not trying to diss links, just trying to find out more about future direction / viability).

And yes, the other big thing is the does-it-all nature...

-- Boris

On Friday 28 July 2006 14:39, Boris Mann wrote:

Branch the sucka! Rip out admin and release it as a feature upgrade!

Advice taken -- I will be doing that ASAP (see my other post). -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/

On 28-Jul-06, at 8:29 PM, Evan Leeson wrote:

On Jul 28, 2006, at 11:39 AM, Boris Mann wrote:

...

And, of course some "how tos" on using it for common scenarios (links directory, etc.) would be good (as MichelleC requested as well in a msg that just came in). It's good to recruit early users to help with this stuff...assign them a task in the issue tracker, make them feel useful!

I'll raise my hand here. I installed this bundle today with every intention of using it, so put me on your early adopter list. I'll be seeing you in the issues queue if anything pops up.

Excellent! Maybe you can come over and present a Links tutorial to the Vancouver Drupal Users Group for August. (I love how bitching and moaning turned into a useful discussion here....long live Links bundle, long live SysCrusher :P) -- Boris

Op maandag 31 juli 2006 04:27, schreef Khalid B:

I have faced a similar problem with sections module.

Then why don't I know from your problems, did I miss a mail you send me that explains why/how/when you want to branch the module? Seriously,had you pinged me once, you (Khalid) would have gotten CVS write access to this module right-away. Feel free to volunteer still! ;) So stop "moaning" and lets turn this particular issue into a useful discussion too. :) Bèr

On Friday 28 July 2006 14:22, Boris Mann wrote:

Would neglected be a better word? 1 & 2 as you mention above are   indicators to me that not enough attention is being paid to the   bundle (I'm not trying to diss links, just trying to find out more   about future direction / viability).

{hanging my head in shame} Okay, here's what has happened with Links... At the time I started work on the module, last summer, I was an IT consultant with a fair amount of idle time on my hands due to the sluggish economy. Just about the time the development got well along, my employer landed a major project that fell right into my lap. It's a series of large upgrades to a Java system totalling well in excess of 100K lines of code -- not trivial, and has provided me with all the billable hours I could work for the past eight months. At about that same time, I took over as the Chief Technical Officer of a large (30K members) nonprofit organization. I hadn't planned for all of this to happen at once, but the result was that for about two months this spring I was averaging 60+ hour work weeks *plus* 10-15 hours per week of volunteer time as the nonprofit's CTO. Something had to give, and it was either Drupal or my marriage or my health -- and Drupal lost. On top of everything else, I maintain approximately ten Drupal sites as a volunteer for other nonprofits such as the public library where my wife works. When those recent security vulnerabilities came out, I had to rush to upgrade a whole lot of sites that I hadn't really the time to do at that moment. Such is life. That being said, I know that I'm *badly* behind schedule on releasing this bundle, and I apologize to the Drupal community for that. I don't know what else to say except that I'm sorry. I haven't lost interest in Links, though, and in fact have started using it on my own sites in production to gain further confidence in its reliability before releasing. I think we're at a point where we can do that now. As Jeff Eaton has indicated, I recently have gotten back into working on Links. I had a long talk with my employer a few weeks ago about those killer hours, and things have eased up quite a bit since then. There have been several contrib patches to Links recently that fixed nagging bugs I didn't have time to address. I like the idea of stripping the admin function out into a separate module -- it's a big chunk of code that (1) is not absolutely required and (2) could be contributed by someone else. I'll do that, and I'll go ahead then and tag the package for 4.7 as soon as this is done. That was a good suggestion, and I'm wise enough to know good advice when it whacks me on the nose. :-) Kind regards, Scott (Syscrusher) -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/

On 28-Jul-06, at 12:29 PM, Michelle Cox wrote:

I don't think you have anything to be ashamed of. LOL! Sounds like you got massively busy. I'm grateful for all the work you devs do on core, contrib, etc and would never presume to take someone to task because they got too busy with other things in their life to work on something for free.

Thanks for doing the module and I'm happy to hear it's moving forward. But don't ever feel guilty because that old RL thing got in the way. :)

Just wanted re-iterate -- +1 to what Michelle says here. I think we got all the clarity we needed about participation and purpose of code. Thanks, SysC -- Boris

Op vrijdag 28 juli 2006 20:22, schreef Boris Mann:

Would neglected be a better word? 1 & 2 as you mention above are   indicators to me that not enough attention is being paid to the   bundle (I'm not trying to diss links, just trying to find out more   about future direction / viability).

No. If you subscribe to the issues, you will find that issues are not only dealt with very fast, but that it gets a lot of good feature requests, often containing patches which are committed rather fast.

And yes, the other big thing is the does-it-all nature..

Also incorrect! It does a lot. But that is because links is made up from about 8 modules. Modules that can all be switched off! It is nothing like ecommerce (wich requires five modules at least). The only one required module is the one containing the API. All other links modules can be used at your wishes, or not! Lets get this FUD out of the way for once and for all: Links is not complex, not bloated and not Big. It is Very Simple. You will find that once you used a module from it, you will deploy it on every site. Even your blog (how cool is it to track if one of the links in one of your blogs points to a none existing site by now?). Bèr

On Friday 28 July 2006 10:40, Matthew Jenkins wrote:

and there's nothing wrong with a little diversity - different ways of looking at the same problems, etc...

For the record, I have absolutely no problem with others building whatever link-handling modules they wish, using or not using my API as they prefer. I'm a firm believer in the Open Source philosophy of letting the user community filter for the best code by voting with their feet. I created Links to scratch my own itch, because there was nothing else that did exactly what I wanted, and am sharing it with anyone else who can use it. There's no reason why we have to have just one links solution, any more than there is any reason why we can only have one theme engine or one photo album manager. The main philosophy behind Links is to have each unique URL stored only *once* in the system, and therefore dead links detection and related admin functions only have to visit each unique URL *once* per iteration. Point solutions are leaner and cleaner if you only need to use URLs in one way on your site (e.g., just a Yahoo-like directory and nothing else). Links API is built to handle the larger-scale situation, and there may indeed be a case to be made for a point solution for the entry-level user who just wants a place to put links and doesn't need to have a scalable management backend for them later. :-) Different strokes for different folks, and all that. Maybe when Links and LinksDB are both done (I don't know the release status of the latter) we can collaborate on a point-by-point feature comparison to help admins choose the right one for their site. Scott (Syscrusher) -- ------------------------------------------------------------------------------- Syscrusher (Scott Courtney) Drupal page: http://drupal.org/user/9184 syscrusher at 4th dot com Home page: http://4th.com/

That's exactly why I created LinksDB as well. To 'scratch my own itch' as you put it. I create dthe module originally for me, and so as not to

Well, if you're going to release LinksDB as a alternative to Links, you should at least attempt to program it correctly. You're not using database prefixes, you're not using Drupal coding style, you're not using db_query properly with %d and %s, you've got _POST all over the place and blah blah blah. At this point, I'd say you'd do yourself some service to actually attempt to use the links package instead, because this code is far from desirable on any type of Drupal installation. And, sorry, I don't buy the "well, it was for me only" schtick - if you're gonna hop on here and disparage Links, intentionally or not, you should at least have some powerful mojo to back you up. You don't. -- Morbus Iff ( black as the devil and sweet as a stolen kiss ) Technical: http://www.oreillynet.com/pub/au/779 Culture: http://www.disobey.com/ and http://www.gamegrene.com/ icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

That code is marginally better, so I'll retract some, but not all, of my earlier comments and scorn.

Morbus I don't think that "scorn" is appropriate here.

On 1 Aug 2006, at 17:55, Morbus Iff wrote:

...

That's exactly why I created LinksDB as well. To 'scratch my own itch' as you put it. I create dthe module originally for me, and so as not to

Well, if you're going to release LinksDB as a alternative to Links, you should at least attempt to program it correctly. You're not using database prefixes, you're not using Drupal coding style, you're not using db_query properly with %d and %s, you've got _POST all over the place and blah blah blah.

You must be looking at a very early release then. I _AM_ using db_query properly (I have been as soon as I discovered that it could be used like printf), and I _AM_ using database prefixes. The _POST stuff I have found no other way around and I am using examples from the drupal sysem! If you can show me a better way then I'll gladly use it, but the documentation is not that easy to work with or find what you want (I know it's hard to make good documentation, and I'm glad for what there is)

At this point, I'd say you'd do yourself some service to actually attempt to use the links package instead, because this code is far from desirable on any type of Drupal installation.

And, sorry, I don't buy the "well, it was for me only" schtick - if you're gonna hop on here and disparage Links, intentionally or not, you should at least have some powerful mojo to back you up. You don't.

At least get your facts straight before you start slating me. And I am _NOT_IN_ANY_WAY_ 'disparaging' links. Just because I have a desire for a different way to do my links than the links module does doesn't automatically make me think it's bad. It's probably a very good module in the right place. That place just didn't happen to be in the environment I was working in at the time. And, as I'm not selfish (like some people seem to be), I am graciously allowing other people to use my code. If you think that's a bad idea then I suggest you go and get yourself a job at Microsoft or something where that kind of mindset is rewarded with large pay rises.

-- Morbus Iff ( black as the devil and sweet as a stolen kiss ) Technical: http://www.oreillynet.com/pub/au/779 Culture: http://www.disobey.com/ and http://www.gamegrene.com/ icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

Scanned by MailDefender - managed email security from intY - www.maildefender.net

Get a life M Information in this electronic mail is confidential and may be legally privileged. It is intended solely for the addressee. Access to this mail by anyone else is unauthorised. If you are not the intended recipient any use, disclosure, copying or distribution of this message is prohibited and may be unlawful. When addressed to our customers, any information contained in this message is subject to intY's Terms & Conditions. Please rely on your own virus scanning and procedures with regard to any attachments to this message. Scanned by MailDefender - managed email security from intY - www.maildefender.net

Wow, Morbus. Will you go through my modules, too, please?

On 2 Aug 2006, at 14:12, Morbus Iff wrote:

...

You must be looking at a very early release then. I _AM_ using db_query properly (I have been as soon as I discovered that it could be used like printf), and I _AM_ using database prefixes.

At least get your facts straight before you start slating me.

Actually, the only reason you're using database prefixes is because you went through and added them all shortly after I sent that message. (?r1=1.1.2.11&r2=1.1.2.12, Wed Aug 2 08:00:47 2006 UTC).

That's when I checked that version in - I had done the changes a long long time ago. OI have also joined head and 4.7 branches

* You've got an empty @file declaration.

Not put there by me - what is it?

* You're passing an unused array of string replacements to t() in linksdb_help().

Copied and butchered from someone else's module - blame them not me.

* You're using HTML in the help of admin/settings/linksdb, which means it has to be translated separately even though it's the same string as the one preceding it. '<p>'.t().'</p>' instead.

Noted - I shall update.

* Your code doesn't follow the Drupal style guidelines at all. http://drupal.org/node/318. Not mentioned there is an in-bred "spaces after commas", which you love to not do.

That's pure prettiness

* You global $user in linksdb_menu() and you don't have to.

The module I learned from had it, and so do I.

* Your use of weights in linksdb_menu(), while they certainly accomplish something, is abnormal. You never define weight 0, default local tasks are set to -10, and the tabs usually are ordered by declaration order, not manually defined weights.

I tried that and it seemed to re-order them alphabetically, which is certainly not what I want when the first one begins with V...

* MENU_NORMAL_ITEM is implied, and doesn't need to be specified.

I didn't know that - thanks, I shall remove them.

* You use "links/category/edit/4" when it really should be "links/category/4/edit". Even with that said, you use arg(3) in your linksdb_categories_edit() when you should be using the naturally passed callback arguments of the menu (where you'd use linksdb_categories_edit($cid)). You use arg() throughout the module when you really shouldn't be.

That's not what I saw when learning - maybe it has changed in 4.7?

* You don't predeclare a bunch of things, such as $form in linksdb_report() - predeclare it as $form = array(); first.

Noted.

* You don't do sanity checking - you assume that arg(2) exists and is a number.

* You spelt "concise" wrong, and your sentence has no period.

Pedant

* You don't know how to use drupal_get_form(). If you give it an ID of "reportform", then it'll happily use "reportform_submit". However, instead of naming it "linksdb_report", you override this automatic/free feature by forcing a callback function of "linksdb_ report_support", which is unnecessary.

The documentation seems to be sadly lacking in how that side of things work - I just used examples and got it working as best I could.

* No standardization in your code. You use $fid, $v for linksdb_ report_submit, $formid, $v in _moderate_submit, and $form_id, $values in your _edit_submit. They're all the same thing.

It depends what mood I'm in and it doesn't affect how the code works in any way. Also, as I learn new tricks in drupal the variables sometimes change with it.

* Your SQL isn't capitalized.

Pedant

* You rarely need to use drupal_goto anymore, and almost never in the _submit hook for FormsAPI.

Except when you don't want the URL to contain the data you passed as pathinfo after you have submitted

* Your SQL queries will not work on PostgreSQL. It needs db_query('word "%s" word') not db_query("word '%s' word").

PostgreSQL is not something I have access to for testing, and have no knowledge of whatsoever.

* You use double spaces after sentences in _report_submit.

I am english. I use english grammar.

* Listen, I know I said you shouldn't use drupal_goto a few bullets ago. That doesn't mean you go and use drupal_set_header with a Location: instead. Man alive.

Does drupal_goto work with external URLs?

* Oh, and you're passing ; at the end of most of your SQL queries. You don't need to do that either.

Force of habbit as it is what I was taught to do on SQL*PLUS back in the early 90's

* You use "'<Root>'" in linksdb_deadlinks, and '<Root>' in linksdb_suggest. Which is it? (For what it's worth, it should be '<'. t('root') .'>', per Drupal core).

If I'd know that I'd have done it.

* To prevent things like "select * from {links_links} where dead=1 order by {links_links}.name", use table abbreviations, like: "select * from {links_links} ll where dead=1 order by ll.name".

Why? I'm happy to type the full name, and it makes it far more readable later. I personally abhor the use of table abbreviations.

* if(variable_get('linksdb_show_report',1)==1) - you don't need ==1.

True, but it sometimes is clearer on the reading of the code if you're specific that you want that value.

* linksdb_deadlinks doesn't translate the report messages.

My bad

* You use "links/report/$link->id" and then 'links/refer/' . $link->id. Which is it? Concatenation or interpolation?

It depends on my mood - they both do the same job.

* Oooh, you do know how to predeclare $form = array() in _suggest(). I guess you must have made sloppy mistakes all the other times. Story of your life, eh?

This is my first module - I am learning the best ways to do it.

* "({name},{url},{description},{active},{category})" - I have no clue what you're even attempting there. Column names aren't prefixed.

yeah, I know, and they're already gone. I was getting a bit overzealous with my prefixing the other day.

* Is it site staff or site administrators? You use both.

Which do you suggest? I'm happy with either.

* theme_link() - Gah, you don't use theme('image') here.

theme whattage? Never heard of it. What does it do?

* Is it "Link" or "link"? Previously you have "Report this link", but then you have "Edit Link".

Pedant

* $cat->shown=99999999;. Right. Heh.

why not? it does the job, and is nice and simple.

* You seem to use the "Report this link as dead" thing a lot. That should be made a function.

Could do I guess.

* # for comments? What is this, Perl?

Why not? they're perfectly valid. And perl is the other language I use day in and day out so I just stick to one comment style.

And so on. But I'm getting bored.

Why not post some issues instead of slating me in public mr 'I'm oh so perfect and have never made even a slight slip in my life when learning a new system'?

-- Morbus Iff ( drowning in data, bereft of knowledge. ) Technical: http://www.oreillynet.com/pub/au/779 Culture: http://www.disobey.com/ and http://www.gamegrene.com/ icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

Scanned by MailDefender - managed email security from intY - www.maildefender.net

Information in this electronic mail is confidential and may be legally privileged. It is intended solely for the addressee. Access to this mail by anyone else is unauthorised. If you are not the intended recipient any use, disclosure, copying or distribution of this message is prohibited and may be unlawful. When addressed to our customers, any information contained in this message is subject to intY's Terms & Conditions. Please rely on your own virus scanning and procedures with regard to any attachments to this message. Scanned by MailDefender - managed email security from intY - www.maildefender.net

On 2 Aug 2006, at 15:46, Morbus Iff wrote:

...

OI have also joined head and 4.7 branches

Noticed - good.

...

...

* You've got an empty @file declaration. Not put there by me - what is it?

I know of nothing that would put that there for you, much less commit the change to CVS. You can certainly remove it without worry, but you should consider filling out your module with documentation:

http://drupal.org/node/1354

...

...

* You're passing an unused array of string replacements to t() in linksdb_help(). Copied and butchered from someone else's module - blame them not me.

No excuse. If you don't fully understand the code you're copying from, you shouldn't be using it.

I agree, but where else am I going to learn from? It's changed now anyway.

...

...

* You global $user in linksdb_menu() and you don't have to. The module I learned from had it, and so do I.

See above.

It's removed.

...

...

* Your use of weights in linksdb_menu(), while they certainly accomplish something, is abnormal. You never define weight 0, default local tasks are set to -10, and the tabs usually are ordered by declaration order, not manually defined weights. I tried that and it seemed to re-order them alphabetically, which is certainly not what I want when the first one begins with V...

Confirmed (by removing all weights from links/edit, links/category, links/moderate, and links/deadlinks, and seeing deadlinks alphabetized before moderate). Interesting - I'd say that's a bug in Drupal, since it doesn't jive with other array usages (such as form elements).

...

...

* You use "links/category/edit/4" when it really should be "links/category/4/edit". Even with that said, you use arg(3) in your linksdb_categories_edit() when you should be using the naturally passed callback arguments of the menu (where you'd use linksdb_categories_edit($cid)). You use arg() throughout the module when you really shouldn't be. That's not what I saw when learning - maybe it has changed in 4.7?

Well, arg() is certainly becoming more and more deprecated, but I believe the use of callback arguments from the menu path has been around since 4.6. In the future, we really hope to not use arg() at all. Regarding the use of "links/category/4/edit", that sort of system (where actions come after the unique entity they relate to) has been around forever, with node/3/edit and user/1/edit, etc.

A short demo using links/category/edit/4 as an example:

$items[] = array( 'path' => 'links/category/edit', 'callback' => 'linksdb_categories_edit', );

function linksdb_categories_edit($a, $b, $c, $d) { }

url: links/category/edit (a/b/c/d empty). url: links/category/edit/1 (a=1/b/c/d empty). url: links/category/edit/1/2 (a=1/b=2/c/d empty).

And so on. To prevent warnings under PHP E_ALL, you'd really want to predeclare your parameters so that uninitialized values don't exist:

function linksdb_categories_edit($a = NULL, $b = 2, ...) { }

This way, if $b isn't passed in (as per our first and second example URLs above), then the code would treat it as if it were set to 2.

If the arguments you need for your function aren't in the right order or some such, you can get more magical with "callback arguments", but it's proper use is too complicated to demo here. For you, you'd want to consider modifying your menus like what is shown in node_menu:

http://api.drupal.org/api/HEAD/function/node_menu

I was hoping there'd be a nice way like that - arg() always seemed like a but of a kludgy hack. I'll see about updating it to use that system for the future.

...

...

* You spelt "concise" wrong, and your sentence has no period. Pedant

Quality assurance and control is not pedantry. Do I believe that things should be spelt and abbreviated correctly, that code should run cleanly under E_ALL, and have some semblance of intelligent design (heh) as opposed to a feeling of continual "adding on"ness? Yes! ;)

...

...

* You don't know how to use drupal_get_form(). If you give it an ID of "reportform", then it'll happily use "reportform_submit". However, instead of naming it "linksdb_report", you override this automatic/free feature by forcing a callback function of "linksdb_ report_support", which is unnecessary. The documentation seems to be sadly lacking in how that side of things work - I just used examples and got it working as best I could.

Are we referring to this one?

http://api.drupal.org/api/HEAD/file/developer/topics/forms_api.html

Ok, not lacking - but bl**dy confusing nontheless.

Anyways, the short answer is: if you name your form "module_bubba", then you automatically get "module_bubba_validate" and "module_bubba_submit" for free. You also get a theme callback for free too. Thus, simply:

function linksdb_report() { ... existing stuff ... return drupal_get_form('linksdb_report', $form); }

and you're already existing linksdb_report_submit() will be called.

Cool - makes it easier to read. The way I understood it was that the first element was passed to the function defined by the third element as the ID of the form.

...

...

* No standardization in your code. You use $fid, $v for linksdb_ report_submit, $formid, $v in _moderate_submit, and $form_id, $values in your _edit_submit. They're all the same thing. It depends what mood I'm in and it doesn't affect how the code works in any way. Also, as I learn new tricks in drupal the variables sometimes change with it.

This is quite laughable.

...

...

* Your SQL isn't capitalized. Pedant

No, it helps visually separate SQL syntax from user data.

I've changed it anyway

...

...

* You rarely need to use drupal_goto anymore, and almost never in the _submit hook for FormsAPI. Except when you don't want the URL to contain the data you passed as pathinfo after you have submitted.

Eh? Say again? If you want to return to "links", just:

return 'links';

The returned value from a Forms API _submit function is treated as the destination URL.

I never knew that. I'll use that from now on then as it's nicer.

...

...

* Your SQL queries will not work on PostgreSQL. It needs db_query('word "%s" word') not db_query("word '%s' word"). PostgreSQL is not something I have access to for testing, and have no knowledge of whatsoever.

Makes no difference. It's a Drupal core standard, and if you really want to argue that twiddling the style of your quotes is something you just CAN'T do because "zomg, it's not ME, y'know?", then you have no right being the maintainer of your own module.

I have reversed all my quotes. It's force of habbit from using MySQL all the time and using "..." to embed variables into the string.

...

...

* You use double spaces after sentences in _report_submit. I am english. I use english grammar.

I am unable to parse if this is English as in American, or English as in "I'm from England". Regardless, however, the double spacing has nothing to do with grammar, and everything to do with typesetting. Ultimately, it's a bad habit picked up from grade school, with teachers who don't actually know WHY they want their students to do that.

See: http://desktoppub.about.com/cs/typespacing/a/onetwospaces.htm http://en.wikipedia.org/wiki/Full_stop#Spacing_after_full_stop

both american sites and meaningless to UK english grammar, which states that after a full stop you have 2 spaces.

...

...

* Listen, I know I said you shouldn't use drupal_goto a few bullets ago. That doesn't mean you go and use drupal_set_header with a Location: instead. Man alive. Does drupal_goto work with external URLs?

Yes, because it passes through to url() underneath. Either way, as per my followup message, you should probably be using header() instead.

...

...

* You use "links/report/$link->id" and then 'links/refer/' . $link->id. Which is it? Concatenation or interpolation? It depends on my mood - they both do the same job.

While I will agree that coding is an art form (I believe in "code shui"), it is how you attack it logically, via design, algorithms, and language that is the beauty representative of your mood, not the chaotic use of inconsistency within your language constructs. The argument that "it doesn't matter, the computer reads them as the same" is irrelevant.

...

...

* Is it site staff or site administrators? You use both. Which do you suggest? I'm happy with either.

I believe internally we use "site administrators" more often.

Then I shall too.

...

...

* theme_link() - Gah, you don't use theme('image') here. theme whattage? Never heard of it. What does it do?

http://api.drupal.org/api/HEAD/function/theme_image

It is the proper way to spit out an <img> via module code. You'd use it something along the lines of:

// just the image itself. theme('image', path_to_theme().'/front_project_placeholder.png')

// an image with a link to node/1234. l(theme('image', path_to_theme().'/front_project_placeholder.png'), 'node/'.$node->nid, NULL, NULL, NULL, NULL, TRUE);

does path_to_theme() return the path to the module? or the path to the theme in use? If the latter, then it's useless to me as the image is in the module not the theme.

...

...

* # for comments? What is this, Perl? Why not? they're perfectly valid. And perl is the other language I use day in and day out so I just stick to one comment style.

perl is the compiler. You mean "And Perl is the other ..." <g> (Sadly, the Perl FAQ indicates it is user choice to make this distinction).

Perl: Practical Extraction and Reporting Language Note the last word that begins with L... Regardless of the capitalisation, perl still means that. And anyway, perl isn't the compiler... perl5.8.8 is the bytecode compiler. perl is merely a symbolic link to it (or it is on sensible systems)

...

Why not post some issues instead of slating me in public mr 'I'm oh so perfect and have never made even a slight slip in my life when learning a new system'?

You'll note that a number of my comments had nothing to do with Drupal, including your chaotic use of concatenation vs. interpolation, "<Root>" vs. "<Root>", and so forth. These are not slight slips when learning a new system, these are just plain old bad habits.

Anyways, to create a patch to "fix" your module would take me roughly an entire day of work, as opposed to the "throwing of the bone" a few 15-minute emails can do. Likewise, these slipups are not bugs, per se, but simply poor self-control, and the best way to fix those are by actually doing them all manually, not via someone else's patch file.

I wouldn't want a patch file. But a more private form of communication would be appreciated.

-- Morbus Iff ( don't heckle the super-villain ) Technical: http://www.oreillynet.com/pub/au/779 Culture: http://www.disobey.com/ and http://www.gamegrene.com/ icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

Scanned by MailDefender - managed email security from intY - www.maildefender.net

Information in this electronic mail is confidential and may be legally privileged. It is intended solely for the addressee. Access to this mail by anyone else is unauthorised. If you are not the intended recipient any use, disclosure, copying or distribution of this message is prohibited and may be unlawful. When addressed to our customers, any information contained in this message is subject to intY's Terms & Conditions. Please rely on your own virus scanning and procedures with regard to any attachments to this message. Scanned by MailDefender - managed email security from intY - www.maildefender.net

...

http://desktoppub.about.com/cs/typespacing/a/onetwospaces.htm http://en.wikipedia.org/wiki/Full_stop#Spacing_after_full_stop

Both american sites and meaningless to UK english grammar, which states that after a full stop you have 2 spaces.

Please give me a reference from a well-respected UK grammar book. Until them, you're non-authoritative and the unspoken Drupal style remains (which is one space after sentences). (Actually, either way, Drupal style is one space after sentences, so you'd have to conform to that anyways to satisfy the Drupal jihadists.) There's a whole discussion about this particular issue at http://en.wikipedia.org/wiki/Wikipedia_talk: Manual_of_Style_archive_(spaces_after_a_full_stop/period) in which they suggest it's not a UK vs. US thing at all (though, I could find nothing authoritative in here that'd satisfy your UK demands). More at http://blogs.warwick.ac.uk/andrewingram/entry/two_spaces_after, which loosely quotes Lynne Truss' book EAT, SHOOTS & LEAVES as "typists were taught to leave two– or even three–space gaps after a full stop until quite recently. Also, semi–colons and colons used to have a space preceding them and two spaces after." I have the book, but was too lazy to find you an exact page reference for that. The Department of Chemistry at Oxford suggests otherwise too: http://www.chem.ox.ac.uk/course/word/msword-21.html And some magazine I know nothing about suggests the same: http://www.pisc.org.uk/ But anyways, I'm all for authoritative sources. What's the name of a respected guide in your part of the world? Over here, we have Fowlers and the Chicago Manual of Style. (And note: don't think I'm picking on you over this. Dare to ask m3vrck about the ellipsis debacle!).

...

// just the image itself. theme('image', path_to_theme().'/front_project_placeholder.png')

does path_to_theme() return the path to the module? or the path to the theme in use? If the latter, then it's useless to me as the image is in the module not the theme.

For that, you'd want: drupal_get_path('module', 'petition') in place of path_to_theme().

...

...

...

* # for comments? What is this, Perl? Why not? they're perfectly valid. And perl is the other language I use day in and day out so I just stick to one comment style.

perl is the compiler. You mean "And Perl is the other ..." <g> (Sadly, the Perl FAQ indicates it is user choice to make this distinction).

And anyway, perl isn't the compiler... perl5.8.8 is the bytecode compiler. perl is merely a symbolic link to it ...

OoOH, snap! :D -- Morbus Iff ( i subscribe to the theory of intellectual osmosis ) Technical: http://www.oreillynet.com/pub/au/779 Culture: http://www.disobey.com/ and http://www.gamegrene.com/ icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

What difference does it make anyway since browsers compress multiple spaces down into one unless they are hard spaces or non-breaking spaces?

This attitude is why your code looks the way it does - you're not interested in knowing the Right Answer, only what works Well Enough. -- Morbus Iff ( were you a community theatre satan? ) Technical: http://www.oreillynet.com/pub/au/779 Culture: http://www.disobey.com/ and http://www.gamegrene.com/ icq: 2927491 / aim: akaMorbus / yahoo: morbus_iff / jabber.org: morbus

Matthew Jenkins wrote:

PHP works well enough, but the right answer is always C.

As a longtime C and C++ programmer, you're wrong. The right answer varies from question to question. I hate perl, and even I'll admit that sometimes perl is the right answer. I like C and C++ and I'll happily admit that they're the wrong answer more than they are the right answer.

Guys Can we get some focus in that discussion please, and not pointless bickering on tangents? Please be on topic, we are discussing the LinksDB module and it, and Morbus was doing a good code review, although the tone has to be less demeaning ... And please show courtsey to fellow Drupallers as well, regardless of how long you or them have been a member ...

Morbus Iff wrote:

But anyways, I'm all for authoritative sources. What's the name of a respected guide in your part of the world? Over here, we have Fowlers and the Chicago Manual of Style.

There isn't one. The Guardian has published its own handbook but as you can see, it doesn't cover things like this at all: http://www.guardian.co.uk/styleguide/page/0,,184844,00.html Neither does the BBC's: http://www.bbctraining.com/onlineCourse.asp?tID=5487&cat=3 I can't speak to the Times's one (the only other with any name recognition) because there's no online reference and it's rather a hidden text. [There's a perception in the UK that US journalism could do with fewer staff-level "style nazis" whose idea of 'factchecking' involves an insane level of needless rewrites and the removal of anything which can't be footnoted, no matter how blindingly obvious the fact may be. This, by way of explanation of why there's no cultural preference for comprehensive style guides here.] The notion that this is a UK/US divide is nonsense. Yes, teachers always told us to use two. But then when I was told that, *everything* typed was monospace. Proportional type doesn't require it, and no typesetter does it since their own local rules prohibit it. The real reason for using one space in web apps is astoundingly simple, however -- only one will ever be rendered by a browser anyway. I think this is therefore moot (and in no small way offtopic). jh (Formerly someone who had to know this stuff professionally) -- ------------------------------------------- John Handelaar E john@handelaar.org T +353 21 427 9033 M +353 85 748 3790 http://handelaar.org ------------------------------------------- Work in progress: http://dev.vocalvoter.com -------------------------------------------

Chris Johnson wrote:

I haven't looked at the code, but it's clear from Morbus' complaints that an equally long list of almost the very same complaints could be made about many of the core files.

Some of Morbus' complaints could have resulted directly from Matthew copying style and technique from core modules. Others are purely religious war sorts of issues, i.e. declaring defaults.

Generally I find this sort of post to be an ill-tempered and nit-picky, rather than useful.

It's generally not a good idea to nit-pick other's code, especially in public, unless you are yourself perfect -- which no one is.

I hereby invite and authorize everybody to nitpick on any code I have ever written, be it in private or public. Preferably by attaching a patch file which points out my errors. There were times I'd have paid a lot of money to get a free lecture like Morbus provided.

And that means it just degenerates into a pissing contest of finding faults with each other. That's not a productive use of time, in my view.

Well, our views could not be more different. Morbus' infuriated letter has resulted in a better module whereas all the bickering by other people has only temporarily filled my inbox. Cheers, Gerhard