[drupal-devel] [task] Remove theme_pager_detail()
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch (ready to be committed) This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. drumm
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Jeremy@kerneltrap.org Status: patch (ready to be committed) The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? Jeremy@kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it.
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: moshe weitzman -Status: patch (ready to be committed) +Status: patch (code needs review) i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used. moshe weitzman Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it?
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Bèr Kessels Status: patch (code needs review) another -1. This theme function was added to allow better themeing of teh pager. Before this function we could only go for the standard pager, now we can theme it much nicer. Bèr Kessels Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? ------------------------------------------------------------------------ Tue, 02 Aug 2005 03:07:38 +0000 : moshe weitzman i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used.
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch (code needs review) Attachment: http://drupal.org/files/issues/pager.inc.diff (1.43 KB) Not sure what happened with the patch file. This is code that isn't used anywhere but is loaded for every page view anyway. It could live in contrib or in some documentation instead. drumm Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? ------------------------------------------------------------------------ Tue, 02 Aug 2005 03:07:38 +0000 : moshe weitzman i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used. ------------------------------------------------------------------------ Tue, 02 Aug 2005 11:24:01 +0000 : Bèr Kessels another -1. This theme function was added to allow better themeing of teh pager. Before this function we could only go for the standard pager, now we can theme it much nicer.
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Dries Status: patch (code needs review) I'm with Neil here. This function is not called anywhere, hence it is dead code -- at least for core and contrib. If you need this function, you can still implement it elsewhere (eg. in your theme). Dries Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? ------------------------------------------------------------------------ Tue, 02 Aug 2005 03:07:38 +0000 : moshe weitzman i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used. ------------------------------------------------------------------------ Tue, 02 Aug 2005 11:24:01 +0000 : Bèr Kessels another -1. This theme function was added to allow better themeing of teh pager. Before this function we could only go for the standard pager, now we can theme it much nicer. ------------------------------------------------------------------------ Tue, 02 Aug 2005 17:44:27 +0000 : drumm Attachment: http://drupal.org/files/issues/pager.inc.diff (1.43 KB) Not sure what happened with the patch file. This is code that isn't used anywhere but is loaded for every page view anyway. It could live in contrib or in some documentation instead.
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: moshe weitzman -Status: patch (code needs review) +Status: patch (ready to be committed) ok, this patch makes sense now. this is simply unused code. moshe weitzman Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? ------------------------------------------------------------------------ Tue, 02 Aug 2005 03:07:38 +0000 : moshe weitzman i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used. ------------------------------------------------------------------------ Tue, 02 Aug 2005 11:24:01 +0000 : Bèr Kessels another -1. This theme function was added to allow better themeing of teh pager. Before this function we could only go for the standard pager, now we can theme it much nicer. ------------------------------------------------------------------------ Tue, 02 Aug 2005 17:44:27 +0000 : drumm Attachment: http://drupal.org/files/issues/pager.inc.diff (1.43 KB) Not sure what happened with the patch file. This is code that isn't used anywhere but is loaded for every page view anyway. It could live in contrib or in some documentation instead. ------------------------------------------------------------------------ Wed, 03 Aug 2005 16:11:01 +0000 : Dries I'm with Neil here. This function is not called anywhere, hence it is dead code -- at least for core and contrib. If you need this function, you can still implement it elsewhere (eg. in your theme).
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Dries Status: patch (ready to be committed) Committed to HEAD. Thanks. Dries Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? ------------------------------------------------------------------------ Tue, 02 Aug 2005 03:07:38 +0000 : moshe weitzman i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used. ------------------------------------------------------------------------ Tue, 02 Aug 2005 11:24:01 +0000 : Bèr Kessels another -1. This theme function was added to allow better themeing of teh pager. Before this function we could only go for the standard pager, now we can theme it much nicer. ------------------------------------------------------------------------ Tue, 02 Aug 2005 17:44:27 +0000 : drumm Attachment: http://drupal.org/files/issues/pager.inc.diff (1.43 KB) Not sure what happened with the patch file. This is code that isn't used anywhere but is loaded for every page view anyway. It could live in contrib or in some documentation instead. ------------------------------------------------------------------------ Wed, 03 Aug 2005 16:11:01 +0000 : Dries I'm with Neil here. This function is not called anywhere, hence it is dead code -- at least for core and contrib. If you need this function, you can still implement it elsewhere (eg. in your theme). ------------------------------------------------------------------------ Mon, 08 Aug 2005 03:04:34 +0000 : moshe weitzman ok, this patch makes sense now. this is simply unused code.
Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: Jeremy@kerneltrap.org Status: patch (ready to be committed) Prior to comitting this patch we had a couple extra lines of code in pager.inc, and a completely alternative possibililty for displaying a pager. For negligible memory, and negligible maintenance cost we had choices for custom themes and modules. Now there is only one type of pager available with Drupal. Sure, someone could write this fuction themselves, but realistically it's just complex enough that very would ever bother to do so. I'm disappointed to see this code removed, as I see no gain, only loss. Jeremy@kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 20:18:04 +0000 : drumm This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. ------------------------------------------------------------------------ Mon, 01 Aug 2005 22:25:10 +0000 : Jeremy@kerneltrap.org The idea is to make it available for use in themes, modules, etc, to design custom pagers. Why exactly do you need to remove it? ------------------------------------------------------------------------ Tue, 02 Aug 2005 03:07:38 +0000 : moshe weitzman i agree with jeremy. can't we find more meaningful patches than ones which strip themeability. I can't evaluate the patch because I don't see a file. maybe thats a bug, or drumm just forgot. not everyone uses our google ripoff pager. i have customized this in the past for clients. you can't look at contributed themes and then conclude that a theme function is not used. ------------------------------------------------------------------------ Tue, 02 Aug 2005 11:24:01 +0000 : Bèr Kessels another -1. This theme function was added to allow better themeing of teh pager. Before this function we could only go for the standard pager, now we can theme it much nicer. ------------------------------------------------------------------------ Tue, 02 Aug 2005 17:44:27 +0000 : drumm Attachment: http://drupal.org/files/issues/pager.inc.diff (1.43 KB) Not sure what happened with the patch file. This is code that isn't used anywhere but is loaded for every page view anyway. It could live in contrib or in some documentation instead. ------------------------------------------------------------------------ Wed, 03 Aug 2005 16:11:01 +0000 : Dries I'm with Neil here. This function is not called anywhere, hence it is dead code -- at least for core and contrib. If you need this function, you can still implement it elsewhere (eg. in your theme). ------------------------------------------------------------------------ Mon, 08 Aug 2005 03:04:34 +0000 : moshe weitzman ok, this patch makes sense now. this is simply unused code. ------------------------------------------------------------------------ Wed, 10 Aug 2005 20:50:30 +0000 : Dries Committed to HEAD. Thanks.
participants (5)
-
Bèr Kessels -
Dries -
drumm -
Jeremy -
moshe weitzman