Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In my Drupal 8 Beta development training I have chosen to practice the upgrade process with the node_clone module. It will advance slowly in small steps.
There is a sandbox to keep track on my changes here.
Comment | File | Size | Author |
---|---|---|---|
#14 | interdiff-2194505-12-13.txt | 17.97 KB | Sutharsan |
#14 | node_clone-D8-2194505-13.patch | 36.95 KB | Sutharsan |
Comments
Comment #1
franskuipers CreditAttribution: franskuipers commentedComment #2
franskuipers CreditAttribution: franskuipers commentedI upgraded the hook_menu into the new routing system.
There are 2 paths:
The original names of the file was clone.*. These are renamed to node_clone.* because I got an error at loading the CloneController class file.
Some observations:
Reviews are very welcome.
Comment #3
franskuipers CreditAttribution: franskuipers commentedlayout change
Comment #4
franskuipers CreditAttribution: franskuipers commentedlayout change
Comment #5
franskuipers CreditAttribution: franskuipers commentedlayout change
Comment #6
franskuipers CreditAttribution: franskuipers commentedThe big patch file....
95% is not migrated yet.
Comment #7
franskuipers CreditAttribution: franskuipers commentedThis file is easier to review with only the files I have touched till now.
Comment #8
Sutharsan CreditAttribution: Sutharsan commentedCopy/paste error: use "clone_method.settings" instead.
Needs to be updated.
Returns a (render) array.
No need to get the raw variables here. $node contains the nid too ($node->getId()) and using $node_token in the controller method will give you the token value. (
cloneContent(NodeInterface $node, $node_token)
).Common usage is, for example:
Don't use t() in classes. Use
$this->t
in forms instead.Yes/No radios are discouraged. Use a checkbox instead.
Use
Comment #9
franskuipers CreditAttribution: franskuipers commentedThis is a working node_clone in the current 8.x-1.x branch of D8 core.
What is working:
Looking forward to the reviews.
PS: the patch is the easy-to-review version.
Comment #10
pwolanin CreditAttribution: pwolanin commentedneed to mark it as needing review
Comment #11
pwolanin CreditAttribution: pwolanin commentedThe patch doesn't look like it applies on top of the 7.x version? The module file is fully added, for example.
I just pushed a 8.x-1.x branch that is a copy of 7.x-1.x
Please post a patch that applies to that branch.
Comment #12
franskuipers CreditAttribution: franskuipers commentedThanks for looking into this, Peter
No the patch was not against the 7.x branch, because then it is impossible (I think) to have a good review.
This patch is against your 8.x-1.x branch.
It is a working version of node_clone in drupal8 8.x branch. I admit there is still a lot to do, but this is a good starting point for further development. When this patch is in I would like to create a number of issues/todos against version 8.x-1.x. The patch reviews per issue will be easier.
BTW: I added a "git format-patch" file if you like to keep my individual commits.
Comment #13
franskuipers CreditAttribution: franskuipers commentedNeeds review
Comment #14
Sutharsan CreditAttribution: Sutharsan commentedPSR-0 to PSR-4 changes applied (https://www.drupal.org/node/2246699).
@franskuipers, you are assigned to this issue. Are you still working on/reviewing this patch?
Comment #15
Sutharsan CreditAttribution: Sutharsan commentedRemove D7 code and/or add @todo comments.
Replace remaining
variable_get()
calls.These configurations only exist within the scope of node_clone. Minimise of modify the config vars. e.g. use
node_clone.method
instead ofnode_clone.clone_method
.The D7 module has more variables. Missing in D8:
clone_omitted
andclone_reset_*
. Should these be added?In my opinion uncommented code is bad. It tends to stick around for ever. At least add a
@todo
or just remove it.Has not been updated to D8.
Add a
@todo
and/or do a quick update.Comment #16
franskuipers CreditAttribution: franskuipers commentedThanks for your patch and review.
I am quite busy these holidays, but will apply this ASAP.
Comment #17
ayalon CreditAttribution: ayalon commentedIs there somewhere a sandobox with the latest version? Can someone create a new dev release so we can work on the port more easily?
Comment #18
pwolanin CreditAttribution: pwolanin as a volunteer commentedPatch doesn't apply to latest 7.x-1.x
error: patch failed: clone.module:1
error: clone.module: patch does not apply
error: patch failed: clone.pages.inc:1
error: clone.pages.inc: patch does not apply
error: patch failed: clone.rules.inc:1
error: clone.rules.inc: patch does not apply
piper:node_clone pwolanin$
Comment #19
pwolanin CreditAttribution: pwolanin as a volunteer commentedI made a 7.x-1.0 release (finally), so now is a good time to start. the existing patch here might have some useful bits, but is rather old at this point.
I just renamed the file and function, and ran drupalmoduleupgrader over it.
Feel free to take the 8.x-1.x branch and start trying to make it work.
Comment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedI have an alternate module I made as part of a work project that does the same thing as Node Clone if someone needs an interim D8 solution. https://www.drupal.org/node/2636002
Comment #21
DamienMcKennaStandardized the issue title.