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.

#3 753224_3_LT_rules.patch763 bytesscor
Members fund testing for the Drupal project. Drupal Association Learn more


mermentau’s picture

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

Priority: Major » Normal

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

scor’s picture

Status: Active » Needs review
763 bytes
hunmonk’s picture

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

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

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

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

sudhirporwal’s picture

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

Status: Needs work » Needs review
sudhirporwal’s picture

Status: Needs review » Needs work

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

hunmonk’s picture

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

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

scor’s picture

@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 files automatically.

hunmonk’s picture

Status: Needs work » Needs review

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


sudhirporwal’s picture

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

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.

sudhirporwal’s picture

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.

shadowhitman’s picture

I still face this issue.Anyone?


sudhirporwal’s picture

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