search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[meta] Drupal 7 fix list #3 #135

Closed maxpearl closed 10 years ago

maxpearl commented 10 years ago

Third of the series. See #132 and #134.

quicksketch commented 10 years ago

As mentioned in https://github.com/backdrop/backdrop-issues/issues/132#issuecomment-30047513, people working on trivial D7 backports do not need to open individual issues for each backport. Instead, Pull Requests can be made directly. Use the drupal.org commit message and everything when filing the pull request. Feel free to even combine multiple backports into a single pull request, as long as each backport is a single commit.

biolithic commented 10 years ago

I'm looking at and working on this issue now.

quicksketch commented 10 years ago

I've merged in both https://github.com/backdrop/backdrop/pull/103 and https://github.com/backdrop/backdrop/pull/101. That's 30 down. Thanks @biolithic!

biolithic commented 10 years ago

Working on this now (1/1)

quicksketch commented 10 years ago

I merged in https://github.com/backdrop/backdrop/issues/111 and marked 13 additional issues in this list N/A, as they apply to modules or files that have been removed from Backdrop.

biolithic commented 10 years ago

as discussed in IRC I am working on taxonomy issues:

2c7f509 | Issue #1525138 by twistor, Georgique: Fixed Illegal string offset 'field' in function TaxonomyTermController->buildQuery().

399cf0a | Issue #1195254 by trrroy, underq, kid_icarus, tim.plunkett, oriol_e9g, xjm: Taxonomy test cleanup.

a80c7e8 | Issue #1549390 by drumm, BTMash: Taxonomy_update_7005() can be faster.

f209013 | Issue #1054162 by tim.plunkett, Dave Reid, joachim, no_commit_credit, Berdir: Taxonomy bundles not supported by EntityFieldQuery.

36541c9 | Issue #1542674 by bas.hr, tim.plunkett, damiankloip: Fixed Wrong count assertion in taxonomy.test.

9749406 | Issue #336697 by oriol_e9g, xjm, jbomb, Davy Van Den Bremt, coltrane, lyricnz: Added Optional vocabulary argument for taxonomy_get_term_by_name().

824eb3c | Issue #687180 by Island Usurper, catch, Berdir, Damien Tournoud, xjm, anthbel: Fixed Deleting a taxonomy vocabulary leaves term reference fields still pointing to it, and a PDO Exception when creating content.

15cd0ca | Issue #814804 by JacobSingh, ksenzee, xjm, draenen, paul.lovvik, David_Rothstein, Gábor Hojtsy, Yorirou: Fixed taxonomy_autocomplete() produces SQL error for nonexistent field.

559c4d7 | Issue #1401496 by bleen18, Albert Volkman: Fixed forum_taxonomy_term_delete() completely broken (takes instead of entity). this weekend

quicksketch commented 10 years ago

Hey @biolithic, I'm working on https://github.com/backdrop/backdrop-issues/issues/154 (converting Taxonomy objects to real objects), which may impact some of these issues. For any problem patch that was submitted before March 23, 2012 that has to do with Taxonomy loading/saving/deleting, we can probably assume that #154 is going to take care of it.

If there was a way to work on other types (Nodes, Users, Files), I'd do that, but those patches are dependent upon the Taxonomy one. Sorry in advance for the toe-stepping that's likely to be going on.

jenlampton commented 10 years ago

I'm going through these issues from old to new, and determining why we had failures before. If the failures were because we already have the patch applied (it was committed to D8 before 2/29) or of it was meant for a module or other bit of code we don't have anymore - I'm marking them as N/A as @quicksketch did.

@biolithic this includes some of the taxonomy patches you were working on.

If the patch needs to merged in, I'm also updating the link to the issue to include the comment with the appropriate patch. Hopefully this will enable us to go back through this issue as a patch-applying machine and knock these out quickly :)

jenlampton commented 10 years ago

I spent some more time today updating the issue. A lot of the older issues were committed to D8 before we forked, so those patches won't apply to us (marked N/A). The closer I got to the top of the list, it seemed like all the patches were relevant, so I stopped going through the issue.

biolithic commented 10 years ago

Thanks to Jen for that updating. I did some investigating (didn't want to screw anything up) and started documenting here https://docs.google.com/spreadsheet/ccc?key=0AmV07dJKoiXDdFBsWV9rRmhKd2JBUUVVSDNCS1VrVXc&usp=sharing so others have a record of what is easy/hard to do in this queue. I'll start committing tomorrow morning smaller issues I can handle tomorrow morning. It looks like some patches apply with p1 where they failed with git apply so I'll take a look and actually put those on a branch tomorrow morning.

biolithic commented 10 years ago

I see @jenlampton "forgot" to todo a patch in this list and I'm not porting it either! (until someone tells me) ;)

jenlampton commented 10 years ago

I can't tell from this list which of these issues are in your pull request. I'm going to go through and attempt to link them up for you now, but in the future can you update the meta issue with the patches that are in your PR so that we aren't duplicating efforts? I'm going to do a new PR today that should have some more of these patches in it :)

biolithic commented 10 years ago

Will update now and try to get done before the meeting. Thanks for noticing, Andy

On Thu, Jan 16, 2014 at 2:31 PM, Jen Lampton notifications@github.comwrote:

I can't tell from this list which of these issues are in your pull request. I'm going to go through and attempt to link them up for you now, but in the future can you update the meta issue with the patches that are in your PR so that we aren't duplicating efforts? I'm going to do a new PR today that should have some more of these patches in it :)

— Reply to this email directly or view it on GitHubhttps://github.com/backdrop/backdrop-issues/issues/135#issuecomment-32537968 .

jenlampton commented 10 years ago

I've updated the meta already. The one thing I was unable to locate was this: applied patch drupal-image_style_failure-1262591-25.patch a68d3e1

Can you please link up that issue (or add it if it wasn't there already)?

biolithic commented 10 years ago

I have in my sanity log that it was in commit Drupal 7 meta 3 #135 from 1 14 from yesterday evening

6adf8ca | Issue #1262592 by jmarkel, pingers, marcingy: Fixed numeric names fail image creation.

https://docs.google.com/spreadsheet/ccc?key=0AmV07dJKoiXDdFBsWV9rRmhKd2JBUUVVSDNCS1VrVXc&usp=sharing#gid=0

jenlampton commented 10 years ago

Thanks @biolithic, meta updated to match. :) Since we now have 3 of us working on these patches, I recommend that as we apply patches to our branches we update each patch with @todo-myname. I'll make mine @todo-jen and Ronnie's as @todo-ronnie.

When we have filed our Pull Requests we can come back here and do a search/replace on the @todo-me with the link to the PR more easily.

Plus, no duplicated efforts :)

jenlampton commented 10 years ago

Hey guys, I think I'm about done for the day. I submitted my PR with about 26 patches, and marked another handful NA either because we already had them from before we forked, or because we didn't need them. I removed the @todo-jen markers and replaced them with a link to my PR. Everything else is fair game :)

There were a few doozies in there (watch out for the remaining @todo items). I updated them in the spreadsheet, and said they would either need to wait till our previous PRs are committed, or need revisiting later for some other reason.

biolithic commented 10 years ago

I am going to work on some patches in this list now starting from bottom todos. Here is a list of what is left in this issue:

af04938 COPYRIGHT.txt does not exist in Backdrop? 552cc18 1 out of 1 hunk FAILED e0961c9 3 of 3 hunks failed efe6bdf 3 of 3 hunks failed 2c7f509 taxonomy 1246bbc confusing, applied then reverted? 1e4c645 modules/simpletest/tests/upgrade/drupal-6.menu.database.php is not a file in Backdrop for patch to apply to 2cd91be 1 of 1 hunks failed b67f7af 2 of 2 hunks failed 3a59ca0 entity.inc moved in backdrop files 46cf232 patch tries to create files which are already in backdrop 09b754a failed function to apply to not in Backdrop 74de092 failed code not in Backdrop - Backdrop uses updated .prop instead e2bab36 already applied to Backdrop f7d5d80 taxonomy 399cf0a taxonomy efd12ac already applied to Backdrop 07e5e3e e3c2e01 8 of 51 hunks failed 0e89940 1 of 1 hunks failed d072581 taxonomy 5c6c07b taxonomy a80c7e8 taxonomy 4777845 failed but easy a121523 doozy f209013 taxonomy 84e34e4 2 of 3 hunks failed e039aaa multiple rewrite (documentation) ec36276 This depends on a previous patch. WIll have to waitl until previous PRs are committed. (Jen) 1569c4b rollback/multiple? doozy

jenlampton commented 10 years ago

Thanks @biolithic! I'm going to start at the bottom and try to wrap up this issue :)

quicksketch commented 10 years ago

There are 11 remaining patches in the issue, most of which @jenlampton has made a start on. I've merged the ones that pass tests and apply cleanly, but the remaining ones need work. To keep the open pull request queue clean, I've closed these. Let's either file new PRs or reopen the existing ones after they've been corrected.

jenlampton commented 10 years ago

The looks like all our patches are in! The only one outstanding was the locale_get_plural fix, which we don't need in Backdrop because we're using the same fix that was added to Drupal 8, in https://drupal.org/comment/5609896#comment-5609896.

quicksketch commented 10 years ago

w00t w00t! 222 patches down! We've made good progress on the other D7 ports as well. Those issues are the next targets. See https://github.com/backdrop/backdrop-issues/issues/132 and https://github.com/backdrop/backdrop-issues/issues/134.