d7ux IA changes require changing admin/site-building to admin/structure and 'Site building' to 'Structure' - we need to find and replace all current paths (don't forget tests).

CommentFileSizeAuthor
#17 sitebuildingrename-521474-17.patch200.57 KBAnonymous (not verified)
#14 sitebuildingrename_521474_1827022_v4.patch200.74 KBJuliaKM
#11 sitebuildingrename_521474_1827022_v3.patch201.55 KBJuliaKM
#9 sitebuildingrename_521474_1827022_v2.patch201.3 KBJuliaKM
#4 sitebuildingrename_521474_1827022.patch201.25 KBJuliaKM
#1 sitebuildingrename_521474.patch5.91 KBJuliaKM

Comments

JuliaKM’s picture

JuliaKM commented
Status: Active » Needs review
StatusFileSize
new sitebuildingrename_521474.patch5.91 KB

The following patch changes the name from Site Building to Structure in:
INSTALL.txt
simpletest.module
system.module
system.test

I wasn't sure whether to change the wording in INSTALL.txt and thought it was better to do so and then have someone else undo it later on.

I'm still really new at creating core patches so please let me know if I made any silly newbie mistakes.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work

OK this is good and nice catch on INSTALL.txt, but we also need to rename all admin/build URLs to admin/structure as well - you should be able to do a global find and replace on the whole Drupal directory for that.

Also, not introduced by this patch, but simpletest now lives at admin/development/testing instead of admin/build/testing - should probably change that since we need to change that line anyway.

JuliaKM’s picture

JuliaKM commented
Assigned: Unassigned » JuliaKM

Thanks for the quick feedback. I'll rename the admin/build to admin/structure and fix the simpletest path this evening and post an update tonight.

JuliaKM’s picture

JuliaKM commented
Assigned: JuliaKM » Unassigned
Status: Needs work » Needs review
StatusFileSize
new sitebuildingrename_521474_1827022.patch201.25 KB

Here's a second try. When I tested the paths after clearing cache locally, I didn't see any remaining legacy admin/build paths.

JuliaKM’s picture

JuliaKM commented

Just wanted to add that the path for simpletest in INSTALL.txt should now be fixed as well.

Status: Needs review » Needs work

The last submitted patch failed testing.

JuliaKM’s picture

JuliaKM commented
Status: Needs work » Needs review

Setting to needs review because the last test never ran.

Status: Needs review » Needs work

The last submitted patch failed testing.

JuliaKM’s picture

JuliaKM commented
Status: Needs work » Needs review
StatusFileSize
new sitebuildingrename_521474_1827022_v2.patch201.3 KB

Here's a re-rolled version of the same patch.

Status: Needs review » Needs work

The last submitted patch failed testing.

JuliaKM’s picture

JuliaKM commented
StatusFileSize
new sitebuildingrename_521474_1827022_v3.patch201.55 KB

Figured out that my code was out of date. Here's another try. Sorry for the confusion.

JuliaKM’s picture

JuliaKM commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

JuliaKM’s picture

JuliaKM commented
Status: Needs work » Needs review
StatusFileSize
new sitebuildingrename_521474_1827022_v4.patch200.74 KB

Here's a fourth try.

Status: Needs review » Needs work

The last submitted patch failed testing.

Anonymous’s picture

Anonymous (not verified) commented

I'm gonna have a looksie in a few hours.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new sitebuildingrename-521474-17.patch200.57 KB

JuliaKM:

I am unable to explain why this patch should work when the previous did not. The tests pass on my setup so far, I'm 20% through and I've surely hit some of the changes with 0 failures. The only difference is the 'diff' lines that occur for every file:

Yours:

diff -p -u -p -r1.72 INSTALL.txt

Mine:

diff -u -p -r1.72 INSTALL.txt

I'm not an expert on what each line in a diff patch mean, so I don't know if this difference of the extra '-p' is significant. I also don't know if you have any control over it. I am making patches with this command from the Drupal root:

cvs diff -up .

It's also possible that the test robot momentarily fell off the wagon!

Let's see what the test bot says about this one.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

When in Paris, can someone take me to a tattoo parlor so I can have etched on my right hand a reminder to change the status to 'needs review' before I press 'save?'

Status: Needs review » Needs work

The last submitted patch failed testing.

Anonymous’s picture

Anonymous (not verified) commented

I am stumped! The patch applies cleanly. The tests succeed on my environment. The test bot blames us for it's failure to run tests. Has test bot gone on strike? Is this some kind of civil disobedience? WHAT ARE YOUR DEMANDS TEST BOT? We are prepared to meet them!

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

Chx showed me how to run tests from the CLI. I can verify that this patch does not break the ability to run tests from the run tests script.

Status: Needs review » Needs work

The last submitted patch failed testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

The test bot uses it's own simpletest script to actually install drupal and set things up before running tests - here's the patch to pifr which I think will stop tests breaking once this is in. Since it's never going to pass the test bot until it's actually in core, marking back to CNR.

#524954: Patch for admin/structure change

dries’s picture

dries commented

Could someone run the tests on their local host and confirm that this patch works? Once approved, this should be ready to be committed.

dries’s picture

dries commented

I'm running the tests locally (60% completed), and I pinged boombatower to be on stand-by to fix PIFR.

JuliaKM’s picture

JuliaKM commented

I am able to apply the bangpound's patch but get the following message during the process:
patching file modules/locale/locale.test
Hunk #1 succeeded at 1057 (offset -1 lines).
Hunk #2 succeeded at 1364 (offset -1 lines).

Does this still count as applying cleanly? Thanks bangpound for all of your help figuring out what was going on!

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD and PIFR has been updated. Thanks!

moshe weitzman’s picture

moshe weitzman commented

Noone even pretends to justify this change in this issue. Thats not right. At least link to the discussion if it happenned elsewhere. Without any link, this looks just like a fiat and we don't work like that. I don't think this change is reduces UX or increases it. It does break thousands of web links and book pages and screencasts.

Anonymous’s picture

Anonymous (not verified) commented

FYI juliakm, the offset notices are the result of HEAD source changes that were committed after the patch was made. The diff patch was "aligned" with line X but the patch applied at line X-1 for two hunks. Success is success even if it's a bit out of step.

Sorry to stray from topic. We now return...

catch’s picture

catch
he/him
Primary language English
commented

I agree with Moshe and take some responsibility for posting a crap issue summary.

Here's brief background (I'm assuming Moshe actually knows where the stuff is and just wants the review process to work normally, but for the sake of anyone else coming in).

Main summary of discussions from the d7ux sprint (which I wasn't at) - http://www.yoroy.com/2009/reorganize-drupal-admin-items-within-d7ux-fram... and http://www.d7ux.org/d7ux-information-architecture-a-detailed-view/

#520444: Use a custom menu for top level toolbar links includes adding an alias for admin -> admin/config, we need to discuss how to make the other paths/sections work both in the toolbar and if it's disabled, I have vague ideas but need to summarize.

A few participants in usability testing have complained about Site building and site configuration being indistinguishable titles (when you configure a module, you're building your site, you build your site by configuring content types, views panels etc. etc.), I couldn't find a d.o. issue for this but one not very good example is http://drupalusability.org/node/50

I/A for config and modules is being worked on at #510110: IA : Configuration & Module - this doesn't include the structure path at all, but it will in the sense we need to move things in/out of structure from config and modules.

Status: Fixed » Closed (fixed)

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

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +IA

Adding tag.