backdrop-contrib / devel

Provides helper functions for Backdrop developers.
https://backdropcms.org/project/devel
GNU General Public License v2.0
10 stars 13 forks source link

7.x-1.5 to 7.x-1.6 cross-ports #49

Open klonos opened 5 years ago

klonos commented 5 years ago

I am assuming that I need to start from 7.x-1.5, since the first release of the Backdrop port claims to have parity with that version: https://github.com/backdrop-contrib/devel/releases/tag/1.x-1.5


git log 7.x-1.5..7.x-1.6 --reverse --no-merges --pretty=format:"- [ ] #xx | [%h](http://git.drupalcode.org/project/[project_name]/commit/%H) | [%s](http://drupal.org/project/devel/issues/" | sed -E "s/^(.*#)([[:digit:]]*)(.*)$/\1\2\3\2)/g"

klonos commented 5 years ago

Hey @quicksketch 👋 ...the D7 module decided to remove support for XHProf in https://git.drupalcode.org/project/devel/commit/dcf77278a96b1275cae5866970048f54c19f34ff and http://git.drupalcode.org/project/devel/commit/9d89e71e19b57a6c55ee78f4ddc14b9a1b55d9da ...they now recommend using https://www.drupal.org/project/xhprof instead. Do you think we should be doing the same in the Backdrop port, or retain support?

See:

https://www.drupal.org/project/devel/issues/1848378 https://www.drupal.org/project/devel/issues/2166165 https://www.drupal.org/project/devel/issues/2471058

Edit: I've filed a PR for it in case you agree to have it removed: https://github.com/backdrop-contrib/devel/pull/64

klonos commented 5 years ago

https://www.drupal.org/project/devel/issues/2653284 | https://git.drupalcode.org/project/devel/commit/66b409593700194623ed486fed58528ab85a68ec is not applicable to us, but that commit implemented "a fix" that was not an actual fix for the problem reported:

Please, remove the string concatenation from menu item's descriptions, because in this format only the first parts of the strings are translatable.

Instead of moving the URLs inside the strings, they should have used @ references, and still use url().

Anyway, that commit made it apparent that menu titles and descriptions were not passed through t() (so would not be translatable anyway), so I have filed a PR that wraps these in t() (and also fixes a few double spaces here and there): https://github.com/backdrop-contrib/devel/pull/71

Edit: same deal with http://drupal.org/project/devel/issues/1169726 and the respective commit in http://git.drupalcode.org/project/devel/commit/496880dc719967db8e31b42add5e368258c70a68

klonos commented 5 years ago

I want to mark http://drupal.org/project/devel/issues/2394661 | http://git.drupalcode.org/project/devel/commit/8f20578af2068c02587ba9a664ef411c38569ae2 as N/A, but wondering re this bit:

https://github.com/backdrop-contrib/devel/blob/1.x-1.x/lib/krumo/class.krumo.php#L676

    $css = '';
    // DEVEL: changed for Backdrop config system.
    $skin = config_get('devel.settings', 'krumo_skin');
    if (!$skin) {
      $skin = 'default';
    }

I see that we ship the various available skins in https://github.com/backdrop-contrib/devel/tree/1.x-1.x/lib/krumo/skins, but I don't see this 'krumo_skin' setting in the admin UI in admin/config/development/devel (and there is no reference to it in https://github.com/backdrop-contrib/devel/blob/1.x-1.x/devel.admin.inc). Am I missing something?

Edit: ...we seem to have removed this option: https://github.com/backdrop-contrib/devel/commit/8d3d66085407bb8816180a7d4c70923e66d592e5