Closed (fixed)
Project:
CVS integration
Version:
6.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Reporter:
Created:
3 Dec 2006 at 20:10 UTC
Updated:
29 Jan 2007 at 08:48 UTC
Jump to comment: Most recent file
OK, I'm posting "work to date". Note, I believe I've done all required to complete the port. However, without having a full test set-up to test every aspect on, I expect some small fixes. But, it's up for review at least.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | cvs_d5_access_and_confirm.patch.txt | 3.41 KB | dww |
| #14 | cvslog_2.patch | 3.07 KB | hunmonk |
| #11 | cvs.d5.patch_3.txt | 54.58 KB | dww |
| #10 | cvs.d5.patch_2.txt | 54.62 KB | dww |
| #9 | cvs.d5.patch_1.txt | 50.98 KB | dww |
Comments
Comment #1
AjK commentedFixing a bug in hook_form_alter() found while testing against project D5 port
Comment #2
AjK commentedI need to roll in http://drupal.org/node/101174 and http://drupal.org/node/101170
Comment #3
AjK commenteddoh, ignore patch in #1, rolled against wrong version. Here's the right one. Still need to roll in Zen's two new patches from the DRUPAL-4-7--2 branch as noted above (hence "needs work" status)
Comment #4
dwweverything from DRUPAL-4-7--2 branch (as of DRUPAL-4-7--2-1) is now in HEAD (http://drupal.org/cvs?commit=47203).
please re-roll this patch against HEAD (since there were a bunch of changes in the last few days, too).
thanks!
-derek
Comment #5
dwwAjK posted a new patch for this here: http://drupal.org/files/issues/cvslog_d5_15_12_2006.patch
(it's in a separate issue, but that's duplicate... let's keep using this one, instead).
thanks,
-derek
Comment #6
dwwthis is broken in a few important ways (mostly bugs in hook_menu()). i'll roll a new patch later, i'm just doing more testing now before i do.
overall, this patch is a great start, don't get me wrong. ;)
thanks,
-derek
Comment #7
dwwhaven't had a chance for the most thorough testing yet, but this at least fixes some of the bugs in hook_menu(). it also includes a cvs.info file.
needs much more testing, but at least it gets past most of the most trivial errors when running it on a live site. ;)
Comment #8
dwwfound a bunch more problems with the crazy FAPI stuff for release nodes. :( i'm working on it.
also, i discovered a core bug in the process: http://drupal.org/node/104280 ;)
cheers,
-derek
Comment #9
dwwyay, my patch for #104280 is already committed. ;) that's nice.
furthermore, with some help from eaton in IRC, i got the evil FAPI stuff figured out. basically, in 4.7.x,
form_builder()was responsible for looking at $_POST directly. in 5.x, it expects everything to be stuffed into '#post' ahead of time. so we just do that ourselves, and all appears to be well.also, the taxonomy hackery was busted, thanks to taxonomy.module revision 1.320:
now, instead of a simple array of options, there's this crazy array of option objects, instead. :(
so, i added a
_cvs_get_option_from_taxo()helper method to search the array of objects for the desired taxonomy option and return it.after fairly heavy (but not exhaustive) testing, all the crazy N-page crap for adding release nodes seems to be working now. ;) a brief survey of other parts of cvs.module's functionality (cvs-application page, repo admin stuff, etc) is also working thanks to the menu fixes also included in this patch.
the final major TODO item is to finalize our plans for an /admin/project block and move everything there. as it is now, you can't easily get to the usual admin/cvs page on your own, since the admin page thinks you just want a cvs block, with only 1 element -- the repo editing pages.
it's just not clear to me what's the best layout for all these pages.
however, once that's resolved, i think this would be fully working now. ;)
cheers,
-derek
Comment #10
dwwafter discussion w/ merlinofchaos in IRC, we decided it was best to use a new /admin/project block for all the project-related admin pages (settings for project, project_release, project_issue and cvs, plus the cvs accounts and cvs repositories pages).
i also discovered a bug in how we were using $may_cache in hook_menu() (since it was screwing up my attempts to get this project block working). i'll back-port that to 4.7.x-* in a separate issue.
as far as i know, this completes the patch. everything appears to be working ok, though more reviews and testing would be most appreciated!
Comment #11
dwwleft a little debugging output in the last patch.
Comment #12
dwwcommitted to HEAD and installed on s.d.o (to make it easier for others to test).
i'm still leaving this issue as "needs review", since i'd appreciate more reviews and testing...
thanks,
-derek
Comment #13
dwwhunmonk pointed out that the "CVS access" tab on project nodes is fubar.
a) there's form_id vs. function name disagreement for "cvs_project_access" vs. "cvs_project_access_form" in the "theme_", "_validate", and "_submit" handlers for that form.
b) the confirm form (at least for deleting access) is fubar. see http://drupal.org/node/64279#confirm_form
Comment #14
hunmonk commentedattached should fix both 'cvs access' tab bugs. not tested
Comment #15
dwwnot quite... sorry, the confirm_form() upgrade docs were a little broken (which i've now fixed). ;)
new patch based on hunmonk's previous one that gets the confirm_form() stuff right. tested locally.
Comment #16
hunmonk commentedcode style looks good.
Comment #17
dwwtested on s.d.o. all good. committed to HEAD as revision 1.134.
Comment #18
dwwoh, but i was still fishing for reviews of the diff for the 5.x port. ;)
cvs diff -kk -up -r DRUPAL-4-7--2 -r HEADthanks,
-derek
Comment #19
dwwComment #20
dwwi'm going to give up on people reviewing these porting patches, so i'm going to mark them as fixed now.
Comment #21
(not verified) commented