Closed (fixed)
Project:
Version Control / Project* integration
Version:
7.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
25 Dec 2010 at 02:21 UTC
Updated:
3 Jan 2014 at 02:41 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
dwwAgreed. And we need a 6.x-2.x branch to exist first. ;) Then a 6.x-2.0 release. Then we can really start thinking about this. But yeah, thanks for opening an issue for this and for curating the list at http://drupal.org/node/1006924
Comment #2
webchickTagging for sprint.
Comment #3
lotyrin commentedStarting on this.
Comment #4
lotyrin commentedAnd here's that. (Hah, and I said I was gonna go to bed.)
I haven't tested this, but I'm able to enable it (and drupalorg_git_gateway which depends on it) without error.
Comment #5
dwwThanks for getting this started! However, there's not much point in working on this until #834252: [meta] Port Project to Drupal 7 is done (or at least a lot further along) since many things are going to completely change in project and project_release...
Comment #6
halstead commented@lotyrin thanks so much for this. It may not end up being compatible with project 7.x but it allowed me to continue porting and testing other modules. It also unblocked updating and testing Drupal.org-Git-Daemons.
Comment #7
halstead commented@dww just said that this module is actually mostly compatible with the future project.module. A few things like hardcoded $type == 'project' need to be replaced with api calls that look up the correct entity types though.
Comment #8
dwwIndeed project_release is still a big unknown. See #1551384: Port project_release to D7 -- it's unclear if we're going to field-ify or not. So, I'd hold off spending any time on versioncontrol_release at this point. Maybe you want to split that out into a separate issue that can be handled on a different timeframe.
But thanks to #1549854: Port project maintainers system all the project maintainer stuff is going to be the same in D7 as it is in D6. And I just skimmed through versioncontrol_project itself and it looks pretty portable at this point. See #1549538: Add a function to determine if a node or node type is a project instead of hardcoded checks on $node->type. Otherwise, I think this should be pretty straight-forward.
Comment #9
senpai commentedNeeded for the Drupal.org D7 Upgrade launch.
Comment #10
drummSpecifically, I see
Notice: Undefined index: project in versioncontrol_release_project_release_form_alter_add() (line 394 of /var/www/dev/release-drupal_7.redesign.devdrupal.org/htdocs/sites/all/modules/versioncontrol_project/versioncontrol_release/versioncontrol_release.module).
on release node add/edit pages like http://release-drupal_7.redesign.devdrupal.org/node/add/project-release/...
Comment #11
drummFor #10 specifically, you probably want project_release_get_release_project_nid(). I made this since the field isn't populated yet on node add.
Comment #12
sdboyer commentedupdating title to reflect meta status.
Comment #13
redhatmatt commentedEstimated 12hr, spoke with Sam Boyer, a bit of unknowns in this issue.
Comment #14
dww- Restoring title from cross-post
- Is this including versioncontrol_project_release? If it includes both, is 12 hours realistic?
- What does it mean to give a meta issue an estimate? Is that for everything the meta issue touches and then we don't estimate the sub-tasks?
Thanks,
-Derek
Comment #15
senpai commentedBumping up an additional 8 hours to a total of 20hr because of versioncontrol_project_release is an unknown, and it deals heavily with formAPI.
Also, the actionable sub-tasks need to be estimated, not this meta issue, and the sub-tasks also need to be cross linked in this issue summary.
Comment #16
drummThis patch changes
versioncontrol_release_ctools_plugin_directory()toversioncontrol_release_ctools_plugin_type()to keep up with the ctools plugin API and adds files toversioncontrol_release.infofor the registry.Comment #17
drummI pushed some code to drupalorg_versioncontrol to work with #16: http://drupalcode.org/project/drupalorg.git/commit/0526d15
Attached is another patch, apply #16 first, which allows the release node add page to load error-free and save the release without too many warnings. I think these two patches are good to commit, but more work needs to be done to line up the form structure, validation, and submission. Most notably, project_release has a Version field which is the full version string; project_release parses this and fills the component fields, like major and minor. I think this module can be simplified a bit.
Leaving assigned to sdboyer since this seems to work well enough for me to use it while working on #1800186: Update package-release-nodes.php to D7 in Drush.
Comment #18
drummI also filed #1878418: Queue release packaging at git code arrival for a new requirement.
Comment #19
dfletcher commentedFound several bugs involving the node API hooks in versioncontrol_project.module.
versioncontrol_project_project_nodeapi_insert()andversioncontrol_project_project_nodeapi_delete()and incorrectly named, so the hooks are never fired.$nodeargument of those insert and delete hooks should not be a reference.versioncontrol_project_project_nodeapi_load()function is entirely superfluous, the correct implementation is already inversioncontrol_project_node_load().Comment #20
dfletcher commentedMessed up the delete function name and fixed up the comments. Rerolled.
Comment #21
dwwThanks for pushing this forward. No time for a thorough review, but based on your comment and a quick skim of the patch, I saw something important. In D7, there's not just 1 node type called "project", so this isn't going to work:
Instead, you want this:
Cheers,
-Derek
Comment #22
dfletcher commentedOk thank you Derek. Rerolled.
Comment #23
dfletcher commentedI started looking at versioncontrol_release and quickly ran into an issue where the code there is expecting a multistep form, which is how it works on the http://drupal.org/node/add/project-release
In my test setup this multistep form isn't happening by default. Does anyone know what module is doing this on the current drupal.org, and is that the same path forward for this form in 7?
I noticed that the project node links to node/add/project-release/[project nid] which might be used to circumvent the multistep when
arg(3)is present. But that doesn't help if someone navigates to node/add/project-release without arg three.I'm in #drupal-infrastructure if anyone wants to discuss this.
Comment #24
dfletcher commentedOk this rolls up the previous patches into one big one.
It contains the following changes:
versioncontrol_release_nodeapi_OLD()into modern equivalents. Had to fix upversioncontrol_release_get_release_label()a bit to get this working, contained a query placeholder that didn't work and was callingdb_fetch_array().versioncontrol_project_user_view()that was breaking user pages. Replaced this query with a db_select() equivalent that joins the correct new tables: the data from project_projects is now in field_data_field_project_type.Comment #25
marvil07 commentedWhile I was taking a look to the repository I saw that many commits by sdboyer and bdragon are already on the
7.x-1.xbranch, so there is some code already ported.Based on that, I guess it is ok to follow including incremental patches.
dfletcher: thanks for joining patches :-)
drumm: I have reviewed/done-manual-testing on versioncontrol_project module changes, and they look fine, but I did not really looked at versioncontrol_release changes in detail, if you (or other person willing to) can review that, I guess we can include the code. Again, many incremental commits in there, so let's follow it.
Some pending bits for the future (uncomplete list):
versioncontrol_project_enable()generic.versioncontrol_project_committers_page()(tablesort_sql()was removed from core in d7).PS: Why is bot not testing this? (7.x-1.x testing is active)
Comment #26
drummYes, incremental changes are the way to go.
Pushed this and removed
@todo db_select(). We don't care if queries are DBTNG or not, as long as they work.Comment #27
drummI also made two small commits I found kicking around on a checkout I had:
http://drupalcode.org/project/versioncontrol_project.git/commit/808197b
http://drupalcode.org/project/versioncontrol_project.git/commit/f2b4386
Comment #28
marvil07 commentedIn the process of taking a look at #1878418: Queue release packaging at git code arrival I found a bug, so I just added the fix directly:
hook_ctools_plugin_type()is needed, but it has a different propose thanhook_ctools_plugin_directory(), so I re-added it.Comment #29
drummI went ahead and made some commits to fix up versioncontrol_release. This module still needs all references to the {project_release_nodes} table replaced with using fields. {project_release_nodes}.tag is now field_release_vcs_label. There is also a reference to the old files table. And some queries that say they need review. All have code comments.
Comment #30
drummComment #31
marvil07 commentedI have added several commits to upstream fixing what is mentioned on comment 29.
Comment #32
drummEverything looks good except for
f4e756a Issue #1007102: Use project instead project_project as node type.
There are multiple project node types with arbitrary names. The query should be
IN project_project_node_types().Comment #33
marvil07 commentedThanks for the quick update, yep, fixed that problem:
Comment #34
drummLooks good. I can't find anything else to fix right now, so let's call this fixed. More specific issues should be opened if found in QA.
Comment #35.0
(not verified) commentedVersioncontrol_project is a bunch of drupal.org-specific glue code between git and project. We need this to port Drupal.org to D7.