Hi

I was just testing the new version of this out (2.0) on a xampp setup on my (Windows VIsta) PC.

The drupal directory is at localhost/forum/

At the migration step the link is wrong. It is saying:

http://localhost/forum/<em>/forum/?q=admin/phpbb2drupal/migrate</em>

There should be no em links in the url, nor the duplication of location. The correct link should be:

http://localhost/forum/?q=admin/phpbb2drupal/migrate

CommentFileSizeAuthor
#3 phpbb2drupal.patch10.2 KBnaheemsays
#1 phpbb2drupal_strings.patch20.19 KBmaartenvg

Comments

maartenvg’s picture

maartenvg commented
Status: Active » Needs review
StatusFileSize
new phpbb2drupal_strings.patch20.19 KB

This is because of a wrong use of the t() function. Nowadays as of 5.x) the placeholders are formatted, and this module uses the emphasize placeholder '%' while '@' or '!' should be used.

I've added an patch which solves fixes the problem.

beginner’s picture

beginner commented
Status: Needs review » Needs work

Thanks for the patch.
You should change only the URLs.

In many places, the themed placeholder is appropriate.

Here is one example among many:
- $out['html'] .= '

  • '. t('Module %module is enabled. OK!', array('%module' => $module)) .'

    • ';
    + $out['html'] .= '

  • '. t('Module @module is enabled. OK!', array('@module' => $module)) .'

    • ';
    In this case, it makes sense to use % and not @ so that the module name can be emphasized (themed) appropriately.

    Can you leave all of those out, and change only where it's required for the URLs?

    Cheers.

    [oh! and let me know if you want to take a turn at maintaining the module :) http://drupal.org/node/145411 ]

    naheemsays’s picture

    naheemsays commented
    StatusFileSize
    new phpbb2drupal.patch10.2 KB

    Attached is an updated patch.

    I do not know too much php, or too much about databases, but I can follow directions and I may try to step to maintain this module.

    First, I want to tackle some "easy" issues, create some patches just to see if I can do the job.

    naheemsays’s picture

    naheemsays commented
    Status: Needs work » Needs review
    naheemsays’s picture

    naheemsays commented
    Title: Imncorrect migration link » Incorrect migration link
    beginner’s picture

    beginner commented

    @nbz: I am not subscribed to the this issue queue anymore, so I don't receive notification of updates. I saw this issue only by chance.

    Even if you are not quite "ready" to handle the job, maintaining the project can be a learning experience for you as it has been for me.
    I have my hands too full with maintaining quite a few other contrib modules. It's someone else's turn to contribute to this module which is currently totally unmaintained.

    If you want to take the plunge, use my contact tab to reach me.

    beginner’s picture

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

    Not tested, but the patch looks ok.

    naheemsays’s picture

    naheemsays commented
    Status: Reviewed & tested by the community » Fixed

    Patch has been committed to head and Drupal-5 branch.

    Anonymous’s picture

    (not verified) commented
    Status: Fixed » Closed (fixed)

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