Module Description

The IPv6 Greeter module defines a new block which can be shown on Drupal sites in order to greet users connecting from an IPv6 address, or to inform the others about the existence of the new protocol.

The content of the block changes depending on the IP address of the user:

For IPv6 Users
Show a greeting message and the IPv6 address of the user
For the others (IPv4 Users)
Show a message and a link to the Wikipedia page about IPv6

The current code can be found in the IPv6 Greeter Sandbox, the module is currently for Drupal 6 only, I will make a 6.x branch to make that clear soon.

The module was first showed on http://ao2.it/53 and then moved to a sandbox project on Drupal.org.

Motivation

The Drupal IPv6 Greeter module has the purpose of raise the awareness that, like it or not, we could be needing IPv6 sooner than expected: the last IANA IPv4 Free pool has been allocated on January 31st 2011 to APNIC, while this does not mean automatically that we have run out of Ipv4 addresses for Internet nodes today, we should be prepared.

Other Modules

There are no modules with similar functionality AFAICS.

Coding Standards

The module has been checked with the Coder module and any issue reported by “Coder” has been fixed.

Notes on the developer

ao2 has contributed some minor fixes and sent patches for Drupal contrib modules before, and has been using git for a long time to collaborate to linux kernel development and other projects, you can see some of his activities in the Open Source ecosystem on this page: http://ao2.it/about-ao2

Comments

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Active » Postponed (maintainer needs more info)

Hello, a02. You need to create a sandbox project, and report here the link.

ao2’s picture

ao2 commented
Status: Postponed (maintainer needs more info) » Active

Hi kiamlaluno, I did that already, it was in the original post as a link.

Anyhow, I am repeating the URL here as well: http://drupal.org/sandbox/ao2/1080006

And it's ao2 with the letter 'o', not a zero :)

Thanks,
Antonio

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Needs work

I wonder a bit about the appeal of this module and whether it is broadly interesting enough to be a full project. That said, popularity/usefulness is not a metric we use for judging modules - just a question to the project creator, really. This is especially true since the module won't work on sites that have page caching enabled and most sites do have page caching enabled.

I created a critical priority issue related to security: #1129324: Prevent XSS from administrator settings.
I created a normal priority issue related to translation: #1129318: Improve translatability.

It also appears that this module doesn't adhere quite to the coding standards. It would be great if you could run the coder module on it with the "minor" setting and see if there is anything to fix.

The critical item is a blocker to approval, the others would be nice to have.

ao2’s picture

ao2 commented

Hi greggles,

I decided to submit IPv6 Greeter for Full Project approval in order to learn more about the Drupal development processes and conventions, I agree it is not incredibly useful as a module, but it is small enough to be a learning tool for me, and a validation tool about my practices for you Drupal heads.

I am addressing your remarks in the relative issue pages, and I opened another one #1131992: Code review about the Code review using the coder module.

At the end of the day even if this would be rejected as a full project I will have learned a little bit more about how a proper Drupal module should look like.

Thanks,
Antonio

alexreinhart’s picture

alexreinhart commented
Component: new project application » module
Status: Needs work » Needs review

Since you've fixed the two issues brought up by greggles, I'm pushing this back to "needs review," assuming you'd still like to get the module published. Someone will try to take a look at it soon.

ao2’s picture

ao2 commented

I also fixed #1131992: Code review and the project is now 'coder' clean.

Thanks,
Antonio

alexreinhart’s picture

alexreinhart commented
Status: Needs review » Needs work

I've looked through the rest of the module and I have just one comment:

In _ipv6_greeter_create_content():
$content .= t("You're not on $ipv6, yet!");

This should use t() to make the substitution, instead of using PHP variable substitution.

Otherwise, everything looks good! You've carefully filtered your input and output and it's a simple module.

ao2’s picture

ao2 commented

@alexreinhart, I a not sure I want to use t() placeholders here, I want the link itself to be translatable (in another language one would to point to a Wikipedia page different than English), and IIUC t() substitutions are applied _after_ translations.

I used PHP variable substitution just because I thought the string would be more readable, maybe I just have to put the link verbatim into the text if this use of PHP variable expansion inside t() is confusing.

Thanks,
Antonio

alexreinhart’s picture

alexreinhart commented

Ah, I see. But the t() documentation suggests this won't work:

Because t() is designed for handling code-based strings, in almost all cases, the actual string and not a variable must be passed through t().

Extraction of translations is done based on the strings contained in t() calls. If a variable is passed through t(), the content of the variable cannot be extracted from the file for translation.

I am not, however, an expert on how t() functions, and so I'm not sure what will happen. You should test this, if you can.

ao2’s picture

ao2 commented

I tested and I can see the $ipv6 variable expanded in the string to be translated.

Because t() is designed for handling code-based strings, in almost all cases, the actual string and not a variable must be passed through t().

Extraction of translations is done based on the strings contained in t() calls. If a variable is passed through t(), the content of the variable cannot be extracted from the file for translation.

I think this description refers to scenarios like t($variable); and not t("string containing a $variable"); which seems to be handled fine (assuming one wants the $variable expanded in the string to be translated).

Thanks for pointing out the issue tho, it made us learn something more about how t() works.

So I think I'll leave $ipv6 here for now. If I am going to change the code again I'll try to use translatable fields to make the block contents completely customizable and in that case using a PHP variable for the link won't make sense anymore.

Regards,
Antonio

alexreinhart’s picture

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

Okay, if it works, I'm happy. Everything else looks good to me.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, ao2! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to alexreinhart for the reviews.

ao2’s picture

ao2 commented

Thanks greggles, I'll take a look at the other applications too, to see if I have any useful comments.

Regards,
Antonio

Status: Fixed » Closed (fixed)

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