In a Drupal 5 installation I noticed that the "optional core modules" group in /admin/build/modules included the xstatistics and comment_notify modules. The latter even included a link to its sponsor in its description. Apparently they both contain "package = Core - optional" in their info file. Is this a normal practice? I was already a bit confused by where some modules were placing themselves in the navigation menu, and this came up like a violation of a last asylum.
If a module that is not packaged with Core is adding itself to the core package, then it is in error and a corresponding bug should be filed. On Saturday 08 September 2007, Cog Rusty wrote:
In a Drupal 5 installation I noticed that the "optional core modules" group in /admin/build/modules included the xstatistics and comment_notify modules. The latter even included a link to its sponsor in its description.
Apparently they both contain "package = Core - optional" in their info file. Is this a normal practice?
I was already a bit confused by where some modules were placing themselves in the navigation menu, and this came up like a violation of a last asylum.
-- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
Larry Garfield wrote:
If a module that is not packaged with Core is adding itself to the core package, then it is in error and a corresponding bug should be filed.
I'll even go so far as to say unkind things about the author of the .info file, given the massive amount of documentation available on what *should* go there.
Quoting Earl Miles <merlin@logrus.com>:
Larry Garfield wrote:
If a module that is not packaged with Core is adding itself to the core package, then it is in error and a corresponding bug should be filed.
I'll even go so far as to say unkind things about the author of the .info file, given the massive amount of documentation available on what *should* go there.
Would it be possible for the package generation process to check for this and abort if the .info file isn't correct? Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On Sep 9, 2007, at 7:28 AM, Karoly Negyesi wrote:
Would it be possible for the package generation process to check for this and abort if the .info file isn't correct?
Am I reading correctly that you just volunteered to write this patch?
If you do this, be sure to fix this first: http://drupal.org/node/109181 "provide feedback to owners when there are errors packaging releases" sadly, there's no mechanism right now whereby the packaging script can propagate errors back to the release owners. if anything goes wrong, the package just doesn't get created and the release node remains unpublished. stuff gets written to the watchdog logs, but the release owners can't read those, only d.o admins. then, i just get support requests about "critical bugs" in the packaging infrastructure, or accusations that cron isn't running (which has also sometimes been true). *sigh* anyway, there's info in the issue (as always) about what needs to happen, someone just has to do the work. cheers, -derek p.s. once there was a feedback mechanism in place, i'd be thrilled for the packaging script to do more sanity checking like this and refuse to create releases for projects that are doing stupid things, such as claiming a contrib is part of core, etc.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Derek Wright schrieb:
On Sep 9, 2007, at 7:28 AM, Karoly Negyesi wrote:
Would it be possible for the package generation process to check for this and abort if the .info file isn't correct?
Am I reading correctly that you just volunteered to write this patch?
If you do this, be sure to fix this first:
"provide feedback to owners when there are errors packaging releases"
sadly, there's no mechanism right now whereby the packaging script can propagate errors back to the release owners. if anything goes wrong, the package just doesn't get created and the release node remains unpublished. stuff gets written to the watchdog logs, but the release owners can't read those, only d.o admins. then, i just get support requests about "critical bugs" in the packaging infrastructure, or accusations that cron isn't running (which has also sometimes been true).
*sigh*
anyway, there's info in the issue (as always) about what needs to happen, someone just has to do the work.
Can't the script just send a mail? Getting uid, name, and mail should be possible. Cheers, Gerhard -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFG5PuYfg6TFvELooQRAugsAJ4uGbco8erO/lBeehOfH/cdcAR2AACfVfa3 FuLLWFKqX2cPpCvD5/zrtVM= =EeCS -----END PGP SIGNATURE-----
On Sep 10, 2007, at 1:08 AM, Gerhard Killesreiter wrote:
Can't the script just send a mail?
No, I don't think that's what we need. However, the issue queue is the right place for this discussion: http://drupal.org/node/109181#comment-303028 Cheers, -Derek
On 9/9/07, Derek Wright <drupal@dwwright.net> wrote:
On Sep 9, 2007, at 7:28 AM, Karoly Negyesi wrote:
Would it be possible for the package generation process to check for this and abort if the .info file isn't correct?
Am I reading correctly that you just volunteered to write this patch?
If you do this, be sure to fix this first:
"provide feedback to owners when there are errors packaging releases"
sadly, there's no mechanism right now whereby the packaging script can propagate errors back to the release owners. if anything goes wrong, the package just doesn't get created and the release node remains unpublished.
Um, do we even have to worry about the packaging script? Couldn't we just check the correctness of the .install file at CVS check-in time? It's possible to provide feedback at that point, and we do know which repository the commit is being made to, as well. Or am I missing something really obvious? ..chrisxj
Chris Johnson wrote:
Couldn't we just check the correctness of the .install file at CVS check-in time? It's possible to provide feedback at that point, and we do know which repository the commit is being made to, as well Do we do any similar checks at CVS check-in. webchick once talked to me about using coder at check-in. If it's possible to run php at this point, I think that some subset of the coder checks could be implemented at this point.
Also, could you please create an issue on the coder issue queue to explain exactly what you're checking for, and I can add it there. -- Doug Green douggreen@douggreenconsulting.com 904-583-3342 Bringing Ideas to Life with Software Artistry and Invention...
Quoting Chris Johnson <cxjohnson@gmail.com>:
Or am I missing something really obvious?
Maybe. The packaging script errors currently aren't captured in a way that can be displayed for the package. This is what Derek wants to correct first. Checking correctness of a package needs to happen when all the parts are together, not in pieces. The packaging script is the logical place to check for the errors in the package as a whole. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On Sep 11, 2007, at 7:20 AM, Chris Johnson wrote:
Um, do we even have to worry about the packaging script?
Yes, but not necessarily to catch this particular error...
Couldn't we just check the correctness of the .install file at CVS check-in time?
.info file you mean. Yes, we certainly could.
It's possible to provide feedback at that point, and we do know which repository the commit is being made to, as well.
Indeed. And, this would be a better place to catch this kind of error (aside from the hordes of bogus .info files already in the repo). By the time someone got far enough to make a tag and a release node for it, it's more of a pain to correct. If our CVS repo won't even let them commit a .info file with bogus stuff, it'll be much better all the way around. On Sep 11, 2007, at 7:40 AM, Doug Green wrote:
Do we do any similar checks at CVS check-in.
Not similar in the sense of checking the validity of the *contents* of any files, but we certainly test validity at check-in time, for example, to enforce all the access control stuff for each project.
webchick once talked to me about using coder at check-in. If it's possible to run php at this point, I think that some subset of the coder checks could be implemented at this point.
Not just possible, already happening. The only difference is you need a copy of the file someone's trying to commit, which isn't something the existing php code cares about. That's certainly easy to do, it just means that bolting on .info file validation or other coder-related checks won't be utterly trivial, it'll require a tiny bit more work (to get a copy of the file to the script doing the tests). And, as with all good ideas, we shall now continue this discussion in the issue queue: http://drupal.org/node/174776 Cheers, -Derek (dww)
Quoting Karoly Negyesi <karoly@negyesi.net>:
Would it be possible for the package generation process to check for this and abort if the .info file isn't correct?
Am I reading correctly that you just volunteered to write this patch?
I don't know how a question suggests I volunteer but if it can be done and with Derek's support maybe. I've already seen and responded to http://drupal.org/node/109181; see you there. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On 9/8/07, Cog Rusty <cog.rusty@gmail.com> wrote:
In a Drupal 5 installation I noticed that the "optional core modules" group in /admin/build/modules included the xstatistics and comment_notify modules. The latter even included a link to its sponsor in its description.
Apparently they both contain "package = Core - optional" in their info file. Is this a normal practice?
Probably a copy/paste error. The module developer started with a core module as the base, and forgot to edit this part out. I was already a bit confused by where some modules were placing
themselves in the navigation menu, and this came up like a violation of a last asylum.
Submit a bug for each of these modules. -- 2bits.com http://2bits.com Drupal development, customization and consulting.
Op zondag 09 september 2007, schreef Khalid Baheyeldin:
Probably a copy/paste error. The module developer started with a core module as the base, and forgot to edit this part out.
No. In case of xstatistics the .info was created WAY before any proper documentation was around. So go ahead. And say unice things about me (i might reply with unnice things about documenting new stuff months later then the features are introduced...) Bèr -- Drupal, Ruby on Rails and Joomla! development: webschuur.com | Drupal hosting: www.sympal.nl
Quoting Cog Rusty <cog.rusty@gmail.com>:
I was already a bit confused by where some modules were placing themselves in the navigation menu, and this came up like a violation of a last asylum.
Any package that doesn't leave the package value empty unless they have a reason based on what has been documented is in error. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
participants (11)
-
Bèr Kessels -
Chris Johnson -
Cog Rusty -
Derek Wright -
Doug Green -
Earl Miles -
Earnie Boyd -
Gerhard Killesreiter -
Karoly Negyesi -
Khalid Baheyeldin -
Larry Garfield