ChiliProject is not maintained anymore. Please be advised that there will be no more updates.
We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.
Refactor lib/redmine/menu_manager.rb to increase extensibility (Feature #269)
| Status: | Closed | Start date: | 2011-03-09 |
|---|---|---|---|
| Priority: | Normal | Due date: | |
| Assignee: |  Eric Davis |
% Done: | 0% |
| Category: | Refactoring | ||
| Target version: | 2.0.0 | ||
| Remote issue URL: | Affected version: |
Description
Currently, the source:lib/redmine/menu_manager.rb is a rather large file containing multiple inner classes and modules of MenuManager as well as patches for rubytree. Concerning Rails' reloading mechanisms, this ties all these things into one big clump.
I was trying to extend the Redmine::MenuManager::MenuHelper from within a plugin, but was repetitively running into reloading issues.
At first the repeated loading of file:lib/redmine/menu_manager.rb patching of the rubytree gem resulted in stack level to deep errors (see #266). To cut a long story short. The only way to fix my problem, is to extract all building blocks of the MenuManager into separate files, to allow reloading them on every request during development.
I am therefore proposing to refactor the menu_manager.rb into separate files and, while we're at it, remove the monkey patching of rubytree.
Associated revisions
[#269] Extracting classes and modules from menu manager into separate files
[#269] Instead of patching rubytree, creating a custom sub class and using that everywhere
History
Updated by Gregor Schmidt at 2011-03-09 03:10 pm
Downside of this approach would be an increased cost when applying redmine patches to the chili sources.
Upside would be, that this refactoring increases the extensibility of chili through plugins.
Updated by Gregor Schmidt at 2011-03-09 03:42 pm
The pull request is sitting at https://github.com/chiliproject/chiliproject/pull/19.
- Assignee deleted (
Gregor Schmidt) - Status changed from Open to Ready for review
Updated by Gregor Schmidt at 2011-03-18 09:24 am
I would be happy to hear any feedback on this topic.
Updated by Eric Davis at 2011-03-18 11:53 pm
Your pull request removed the bundled gem without adding it's requirement back in anywhere (e.g. no config.gem). I think removing the bundled gem should be separate from splitting the classes.
Updated by Gregor Schmidt at 2011-03-19 09:19 am
Eric Davis wrote:
Your pull request removed the bundled gem without adding it's requirement back in anywhere (e.g. no
config.gem).
The config.gem line is in config/environment.rb ever since. Therefor there was no need to add it.
I think removing the bundled gem should be separate from splitting the classes.
I totally agree with that. I was a bit over-the-top with that one. Sorry for that. I've edited the request accordingly.
Thanks for taking the time and giving the feedback.
Gregor
Updated by Eric Davis at 2011-03-19 07:39 pm
Gregor Schmidt wrote:
The
config.gemline is inconfig/environment.rbever since. Therefor there was no need to add it.
My mistake, sorry. I forget that vendor/gems aren't autoloaded like plugins.
I've merged your changes into unstable. I like what you did and would like to see more of lib/ split like that.
- Target version set to 2.0.0
- Assignee set to Eric Davis
- Category set to Refactoring
- Status changed from Ready for review to Closed
Updated by Gregor Schmidt at 2011-03-19 09:24 pm
Thanks. I'm happy to help.
Gregor
Updated by Gregor Schmidt at 2011-03-24 09:52 am
For future reference, if anybody happens to come here using search or something:
The gem line in config/environment.rb which defines the dependency to rubytree is not specific enough. It should be something like
config.gem 'rubytree', :lib => 'tree', :version => '~> 0.5.2'