Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

On Saturday I branched and made an initial commit for the port. There's still a long, long way to go.

sun’s picture

I'd be very happy to work on this.

sdboyer’s picture

This, plus the port of Panels, is my big goal for the Vancouver sprint. I'm going to try to make headway on it prior to the sprint, though, so that I can really just have a task list and gun through while I'm there. Maybe even farm some of it out :)

sun’s picture

Assigned: Unassigned » sun
Priority: Normal » Critical
Status: Active » Needs work
FileSize
7.05 KB

Just let you guys know that I started...

sun’s picture

FileSize
38.93 KB

Second pass.... need a break - my eyes are bleeding.

but also, FWIW, first time looking at ctools_modal_form_wrapper() makes me worry.

Pasqualle’s picture

some doxygen changes
update_x() functions removed
more use of drupal_static()

can we have a list of files which will be removed? I guess the ajax and the advanced form functionality is in core.

sun’s picture

@Pasqualle: Thanks for trying to help! However, at the moment those changes make no sense.

- Unnecessary changes like (unrelated) doxygen changes, we always leave out in ports.

- Update functions need to be ported, not removed; keeping the _identical_ functionality.

- I already double-checked the statics and drupal_static() makes no sense there.

Please wait with such changes until this issue (basic port to D7) has been fixed.

sun’s picture

Status: Needs work » Needs review

Quick.

I don't understand the purpose of ctools' includes/menu.inc, even after comparison to core's, but to make any sense, we need to move any tweak in there into core - now.

If there is any tweak.

sun’s picture

FileSize
45.04 KB

Just to post something in between. I keep on having those files open in my editor since I started.

#353208: Allow for runtime-conditional local tasks (hook_menu_local_tasks_alter()) now contains a trivial, but mission critical patch to remove yet another include file from ctools.

sun’s picture

Alright, while working on this, I realized that I was doing each one of the major upgrade steps separately in one fell swoop (instead of going through all files and nitpicking all changes for D7 in every single line).

Hence, I changed the strategy and started to commit in atomic changes:

#589636 by sun: Updated for automatic database schema installation.
#589636 by sun: Updated for all 'class' attributes are an array.
#589636 by sun: Updated .info files for D7 and new class registry.

DBTNG, File API, and hook changes are already on the way.

But, ugh, when looking at the commit messages afterwards, we have a problem:

#580342: Fix Page Manager variant import, which did not work at all.
#589342: Introduce page_manager_get_current_page() to get information about the current page manager page.
#564492: by Roger Lopez: Change default class includes from .inc files to .class.php files so they don't get read as plugins.
#590654 by fenstrat: Add submit form as a possible AJAX operation.
#534034 by DamienMcKenna: Add access rule for term ID selection.
#531522 by alex_b: Introduce ctools_static() modelled after static object handling in D7.
#541428 by viz8: User name as a context argument.
#592692 by johnskulski: use theme_links instead of theme('links') for dropdown because we do not actually want a theme to change this output.
#547686: Allow view panes to better respect the default pager settings.
#424548: Add a warning about overridng pager settings with Views AJAX.
#555802: Add an administrative title to custom content panes to make them easier to manage.
#556870: Node edit settings form context autocomplete was not working correctly.
#491884 by ayalon: Provide blank substitutions for optional contexts that do not appear.

These were not committed to HEAD, so, uhm, what do we do about that? I certainly can try to merge all those changes from DRUPAL-6--1 into HEAD now (ideally by trying to apply the patches to catch potentially changed code in already upgraded code), but I'm not sure how we want to proceed after this one-time merger.

sun’s picture

FileSize
39.97 KB

DBTNG + File API + some smaller changes. I'll extract those too into separate commits (easy, no worries).

However, before proceeding with commits, we need to do something about #10. Another possible option would be to completely replace HEAD with DRUPAL-6--1 again and merge all changes manually in there -- most probably that approach would definitely catch all changes from DRUPAL-6--1, so we wouldn't have to worry later on.

Thoughts?

merlinofchaos’s picture

I'll find some time and try to merge the changes from #10 into HEAD. I think it'll take less time than starting over. At least one of those changes actually is in HEAD (the ctools static). A couple of the changes don't apply anyhow, and a couple of them are just new .inc files.

sun’s picture

FileSize
45.89 KB

Thank you! I've seen you committed some - are those all?

Re-rolled against HEAD + some further DBTNG code I either forgot or has been introduced by those commits.

merlinofchaos’s picture

There's a couple that I still have to do because they're fairly tedious and I didn't make patch files so I pretty much either have to do them by hand or extract patches out of webcvs which is kind of a pain.

sdboyer’s picture

Please please please don't invest manual effort in reintegrating those patches. It's FAR easier to do in git. In fact, I'll see if I can't just do it tonight...

sun’s picture

FYI: Also created #599640: Move hard-coded comment query options to caller to remove another bloat of duplicated core lines from ctools.

dmitrig01’s picture

FileSize
47.8 KB

This gets it so you can enable the module with fewer errors:

sdboyer’s picture

re: #15, the list of patches from above has been merged into HEAD. There were only four conflicts, all quite minor, and I'm pretty sure I resolved them correctly.

sdboyer’s picture

Oh eew, forgot the other part of making it easier - rerolling the patch so it applies to the updated tree. Will do tonight.

sdboyer’s picture

FileSize
53.77 KB

I keep feeling like little pieces are probably getting borked, but...here's the re-updated patch file.

DamienMcKenna’s picture

FYI, a related task for Panels 3: #605944: Drupal 7 port of Panels 3

DamienMcKenna’s picture

This module isn't in the D7CX group, is that just from lack of available hours in the day? I presume some additional help would be appreciated? :)

merlinofchaos’s picture

No, I'm not on board with the d7cx movement and I am unwilling to make commitments like that.

sdboyer’s picture

@DamienMcKenna - I'm making the general effort to see that we're ready per on D7 launch day, but am also unwilling to actually make the formal commitment.

sdboyer’s picture

OK, doing this with patch files is just silly and less maintainable. I've committed in the patch file, along with some more additions to what was already done. I'd rather we actually commit in with informational messages, and I can easily merge in forward-ports of changes to the 6--1 branch (if they don't 'just work'). If you don't have commit access, feel free to post incremental patches to this issue, or file separate issues; I'll integrate them as needed.

dmitrig01’s picture

FileSize
41.31 KB

I did a bunch of upgrading through coder - but i haven't upgraded plugins or page_manger yet.

merlinofchaos’s picture

else if --> elseif is not an upgrade. It's a style correction. Please do not include style corrections in upgrade patches.

sun’s picture

I agree with Earl -- that absolutely doesn't belong into a porting patch/effort. (Though I really like those changes, so please keep 'em somewhere ;)

+++ plugins/access/perm.inc	18 Oct 2009 23:18:45 -0000
@@ -27,12 +27,10 @@ function ctools_perm_ctools_access() {
-  foreach (module_list(FALSE, FALSE, TRUE) as $module) {
+  foreach (module_implements('permission') as $module) {
     // By keeping them keyed by module we can use optgroups with the
     // 'select' type.
-    if ($permissions = module_invoke($module, 'perm')) {
-      $perms[$module] = drupal_map_assoc($permissions);
-    }
+    $perms[$module] = drupal_map_assoc(module_invoke($module, 'permission'));
   }

I guess this won't work due to the new structure of permissions.

I'm on crack. Are you, too?

sdboyer’s picture

I had the same reaction when I initially saw coder complaining about 'else if' to 'elseif'. There IS a slight difference, though; however, that only affects the colon syntax, which we never use.

I think it's worth filing an issue in coder - at minimum, it should be demoted to a 'minor' violation.

merlinofchaos’s picture

Yeah. The 'elseif' 'else if' debate is kind of annoying. As a C programmer originally, I default to 'else if' since C and variants do not have 'elseif'. So it's naturally 'else if' throughout my code. And it seems silly to have a coder style rule about it, since the presence or absence of the space doesn't affect readability at all.

dmitrig01’s picture

ok, sorry, i was just trying to make coder shut up.

dmitrig01’s picture

FileSize
41.31 KB

Ok, this shoudl be better

dmitrig01’s picture

FileSize
19.09 KB

old patch

sdboyer’s picture

Sorry for the delay, I'll try to integrate these tonight or tomorrow night - I've VERY nearly got the new tailor/bzr automated setup done!

Dave Reid’s picture

Issue tags: +D7 porting

Adding D7 porting tag...

Nick Lewis’s picture

What do you say we open this back up? Seems like a good goal for this patch would be to get thinks working enough that ctools doesn't break the site. After which, we have a branch?

sdboyer’s picture

Yeah, now that the video has made its rounds... :P

There's some fun merging that still does need to be done to catch HEAD up to where it needs to be. Fortunately, I actually have the tools together now to be able to do that merge properly.

Nick Lewis’s picture

Okay, I guess then any work should wait that patch. Just let me know how I can help beyond reviewing it once posted.

sdboyer’s picture

I'll be committing it to the HEAD branch, at which point I'll ask folks to post their patches to here as diffs against HEAD. It's too hard to keep up with merging in the changes we're making on DRUPAL-6--1 in just a patchfile.

sdboyer’s picture

Code has been merged in to HEAD, that's now our 7.x dev branch.

Nick Lewis’s picture

So tomorrow night, shall I get the current patch synced with head, and report back - or were portions of this patch already put in? I confess I'm only a foot soldier, and not an expert in this system and do not understand whether the comments above from October are still relevant. Marching orders will be followed however.

Or i'll try to work this out. Either way, my sense is that panels is a really *the test case* for so much of this stuff that i'm wondering how i'll tell its working if i'm upgrading this module... Not breaking d7 is a good first step i guess however lol. Can't wait to see how all those API changes from 6 to 7 effect ctools.

zeropaper’s picture

#40 is an awesome news, looking forward.
(btw, I really would like to help...)

Nick Lewis’s picture

So this is still assigned to sun. Wanted to know what the deal was: should someone grab the torch of moving the patch forward? Or is someone already doing lots of work somewhere in an undisclosed location and will be posting the first patch shortly?

merlinofchaos’s picture

FYI I am not currently able to work on this at all until after the next releases of Views 3 Panels/CTools/PanelsEverywhere.

sdboyer’s picture

Assigned: sun » sdboyer

Forgot to reassign. Feel free to post patches.

merlinofchaos’s picture

Status: Needs review » Active
mikeryan’s picture

In what shape is the basic plugin manager support? It seems to me that it shouldn't take very much at all to work on D7...

Thanks.

sdboyer’s picture

@mikeryan: Plugin management is pretty much entirely unaltered, at least for procedural plugins. OO plugins may be a different story, depends on how we integrate with the registry.

mainpc’s picture

klausi’s picture

There is a port on github: http://github.com/hugowetterberg/ctools

voxpelli’s picture

In response to #54 - that repository is just a very little quickfix to get exportables working so that the work on porting Services to Drupal 7 could begin

Remon’s picture

subscribe

Anonymous’s picture

subscribe

Anonymous’s picture

Subscribe

davebv’s picture

+1

chawl’s picture

subs

kika’s picture

subscribe

rfay’s picture

subscribe

Jackinloadup’s picture

subscribe

klausi’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Active » Fixed

D7 version is in CVS HEAD now.

sdboyer’s picture

Status: Fixed » Active

Uh. Not even a little fixed.

bpeter’s picture

Subscribing

Daniel Norton’s picture

++

mahfiaz’s picture

Subscribing

Averiz’s picture

Im just a lowly theme designer with little php knowledge. Panels is a must have module. So I'm happy to help with testing or gui input. Or whatever else I can do to further the project along.

sdboyer’s picture

6.x-1.x has been rolled back into HEAD.

jennycita’s picture

subscribe

FiNeX’s picture

subscribe

chichiMi5’s picture

subscribe

gaelicWizard’s picture

sub.

HEAD seems to report version 6. Where do I get the (in progress) code for 7?

Pasqualle’s picture

Remon’s picture

@Pasqualle, cvs release is a lil bit old. Try this one http://github.com/sdboyer/ctools

sdboyer’s picture

Status: Active » Fixed

Closin this sucker out in favor of individual issues, now that we've just about wrapped the code sprint.

Once I'm back from vacation (the 27th), I'll be remerging the changes made in the repository noted in #80 back into HEAD.

Dave Reid’s picture

So, can we have a 7.x-1.x-dev release yet?

merlinofchaos’s picture

Not until our git workspace is merged back into CVS next week.

groovehunter’s picture

sub

BenK’s picture

Subscribing...

rwohleb’s picture

subscribing

michaek’s picture

7.x-1.x-dev?

iaminawe’s picture

subscribe

Dave Reid’s picture

Status: Fixed » Active

Is exporting back to CVS really this big of a pain? I'll volunteer to pull from someone's Git and commit to CVS if desired. Re-opening as the project does not contain any of the new recent code yet.

rfay’s picture

It would be nice to have a release of some type, as I know that several modules now depend on (some) github version of this, but it's not easy to figure out what version one should use.

merlinofchaos’s picture

Status: Active » Closed (fixed)

sdboyer said he was going to do this on Friday. It hasn't happened.

But re-opening this issue is interpreted as yelling at us, and that's not appreciated giving the assload of work we've done over the last month. Please be patient.

Also, closing and locking this issue. It's not at all useful, other than spamming my inbox with +1 subscribes.