this module needs to be checked for 7.x compatibility, and updated if necessary.

7.x development for LoginToboggan happens on the HEAD branch, so please commit any 7.x changes there.

CommentFileSizeAuthor
#3 753224_3_LT_rules.patch763 bytesscor

Comments

mermentau’s picture

mermentau commented
Priority: Normal » Major

Tried to install Login Toboggan Alpha 3 on Drupal 7 Alpha 6 and get this warning today:

Integrates LoginToboggan with Rules module
This version is not compatible with Drupal 7.x and should be replaced.

hunmonk’s picture

hunmonk commented
Priority: Major » Normal

drupal 7 isn't released yet, this is not a major issue at this time.

scor’s picture

scor commented
Status: Active » Needs review
StatusFileSize
new 753224_3_LT_rules.patch763 bytes
hunmonk’s picture

hunmonk commented
Status: Needs review » Postponed (maintainer needs more info)

the module was ported prior to the initial 7.x release. can you give me some more detail what your patch is addressing exactly?

scor’s picture

scor commented
Status: Postponed (maintainer needs more info) » Needs review

This are the changes I had to make so that LT can integrate with the latest -dev snapshot of rules. Rules complains that # Unable to add this event as it does not provide all variables utilized by the reaction rule unlabeled. otherwise. This patch was rolled against the LT 7.x (git master branch).

hunmonk’s picture

hunmonk commented
Status: Needs review » Postponed (maintainer needs more info)

sudhirporwal needs to weigh in on this -- LT Rules is his submodule...

1sp’s picture

1sp commented
Status: Postponed (maintainer needs more info) » Needs work

I see the patch as incomplete because it misses the necessary changes (explicit loading of required files in .info etc.)

scor’s picture

scor commented
Status: Needs work » Needs review

http://drupal.org/node/542202#files

1sp’s picture

1sp commented
Status: Needs review » Needs work

I believe that in .info file you would need to load up the logintoboggan_rules.rules.inc file. Please consult the rules module code for details.

hunmonk’s picture

hunmonk commented

you're only required to include php files in the .info file if they contain class or interface declarations, which logintoboggan_rules.rules.inc does not.

i am, however, a bit confused how that file ever gets loaded. does rules scan for [modulename].rules.inc files or something?

scor’s picture

scor commented

@sudhirporwal .info does not load up the files for you, unless they contain classes in which case they need to be listed in .info.

rules.module includes the MODULENAME.rules.inc files automatically.

hunmonk’s picture

hunmonk commented
Status: Needs work » Needs review

@sudhirporwal: now that that's cleared up, can we get a review on the actual changes in the patch?

thanks!

1sp’s picture

1sp commented
Status: Reviewed & tested by the community » Needs work

@scor ok, i understand that. But I believe this is something that is followed by all Rules extending modules I have seen for Drupal7, we should follow the convention if any. @hunmonk, what do you think on this ?

P.S I tried the patch and it does work with latest Drupal7, LoginToboggan-7.x and Rules-7.x releases. We just clear out the .info matter and I will commit it into the code-base.

hunmonk’s picture

hunmonk commented
Status: Needs review » Reviewed & tested by the community

@sudhirporwal: i want to follow core conventions, not Rules module conventions. most likely those other modules are not doing it correctly, as at one point it was required to put all includes in the .info file, but that was amended during the development cycle.

so, this is ready to go.

1sp’s picture

1sp commented
Status: Needs work » Fixed

Very well, here it goes to the LT master branch :)

@hunmonk : thanks for the tip regarding module includes.
@scor cool work. thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

shadowdknight’s picture

shadowdknight commented

Hi,
I still face this issue.Anyone?

Thanks

1sp’s picture

1sp commented

@shadowhitman can you please elaborate the issue you are facing ?