I ran the 6.x-2.x code through http://upgrade.boombatower.com/ and this is what I got. Needs a bit of reviewing, but generally looks good.

CommentFileSizeAuthor
#6 995446-05-upgrade_boombatower_com-d7.patch4.69 KBFreso
#1 995446-01-upgrade_boombatower_com-d7.patch4.63 KBFreso
languageicons-d6-d7-update.patch13.83 KBFreso

Comments

Freso’s picture

Freso commented
Status: Active » Needs review
StatusFileSize
new 995446-01-upgrade_boombatower_com-d7.patch4.63 KB

I still need to figure out whether to keep the suggested "admin/config/language/icons" instead of the current "admin/config/regional/language/icons". Probably not, but I'm having some issues with my development server, so... :/

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Shouldn't all instances of function languageicons_update_6xxx() change to function languageicons_update_7xxx()?

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

1. I think that it's more logical for the menu item to be placed in Regional settings -> Languages

2. The menu item should read 'Language icons' and not plain 'Icons', because there are a lot of projects out there aiming to add icons in various parts of the interface like in admin menu, config page etc. This would avoid future confusion.

Freso’s picture

Freso commented

Shouldn't all instances of function languageicons_update_6xxx() change to function languageicons_update_7xxx()?

Nope: Never renumber update functions. (hook_update_N documentation)

1. I think that it's more logical for the menu item to be placed in Regional settings -> Languages

Yep, me too. I just didn't know why the script wanted to place it somewhere else, and since I couldn't check it out on a test site, I decided to let it stay until I'd tried it out. Which I have now. ;) It is back in its proper menu location now. :)

2. The menu item should read 'Language icons' and not plain 'Icons', because there are a lot of projects out there aiming to add icons in various parts of the interface like in admin menu, config page etc. This would avoid future confusion.

Hm. If this would be a source of confusion, then I believe the string should rather have a context added to it. Since the menu item is placed underneath "Language", it would be superfluous to have "Language icons" as a child of it... (IMHO) Either way, this is a new issue which in all cases should be opened on its own - feel free to do so, to open the discussion for everybody to join in on. :)

Freso’s picture

Freso commented
Title: Update the 7.x porting through upgrade.boombatower.com » Update the 7.x porting guided by upgrade.boombatower.com
Status: Needs review » Fixed

The attached patch has been committed. Apart from the changes from upgrade.boombatower.com, I also removed a variable_get() NULL default value and update the @todo for languageicons_update_6200().

Freso’s picture

Freso commented
StatusFileSize
new 995446-05-upgrade_boombatower_com-d7.patch4.69 KB

And here's the committed patch. :p

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented

Hey Frederik, I somehow missed your replies in #4. I've created separate issues for these:

#1003152: Update numbers for the 7.x branch should be numbered 71xx (according to module's target core and major version number)
#1003260: Language icons link should be under Configuration -> Regional settings -> Languages
#1003278: Link text should be 'Language icons' & configuration page title should be 'Language icons settings/configuration'

Status: Fixed » Closed (fixed)

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