Refactor packet config #50
Conversation
update_limits_items_cache. Update specs. #34
| @@ -87,6 +87,7 @@ spec = Gem::Specification.new do |s| | |||
| s.add_development_dependency 'reek', '~> 1' | |||
| s.add_development_dependency 'roodi', '~> 4' | |||
| s.add_development_dependency 'guard', '~> 2' | |||
| s.add_development_dependency 'wdm', '>= 0.1.0' if Gem.win_platform? | |||
There was a problem hiding this comment.
This was so Guard didn't complain.
There was a problem hiding this comment.
You can't do this. Dependencies are only evaluated when the gem is created, so this will break other platforms if the gem is created on windows.
There was a problem hiding this comment.
You can do something like this in the main Gemfile. What is requiring wdm that is not installing it?
There was a problem hiding this comment.
Note approval is dependent on removing this.
There was a problem hiding this comment.
Guard wants it on Windows so it doesn't have to poll the filesystem. I'll add it to the main Gemfile.
| @@ -569,6 +583,7 @@ def reset | |||
| def clone | |||
| packet = super() | |||
| if packet.instance_variable_get("@processors") | |||
| packet.instance_variable_set("@processors", packet.processors.clone) | |||
There was a problem hiding this comment.
Not properly cloning the processors was a bug and I added a spec for it
|
Approved. Big refactorings like this are scary, and almost guaranteed to lead to regressions. In general, let's make sure we only refactor things that truly need improvement and can be made simpler, not just try to make Code Climate happy. |
|
I agree that it is a lot of changes but each time I do this I catch a few bugs as well. Plus these mega-classes make the code very unapproachable for new people who want to understand and contribute. |
|
Approved |
|
Is this done? |
|
I was on vacation. I have a few more spec fixes and then it's done. |
|
Changes Unknown when pulling bf65756 on refactor_packet_config into * on master*. |
|
Changes Unknown when pulling bf65756 on refactor_packet_config into * on master*. |
|
@ryanatball please review |
| # variables. | ||
| # | ||
| # @return [Array<String>] Warning messages for big definition overlaps | ||
| def check_bit_offsets |
There was a problem hiding this comment.
This was previously in packet_config but I felt like it belongs here inside the Packet class.
|
Approved. |
|
All the specs pass and when I run individual specs in the packets/parser folder I get 100% coverage. You might notice that as I trimmed down the packet_config code I also trimmed it's spec and moved stuff into the individual parser specs. |
Merge in COSMOSEE/base from COSMOSEE-127_settings-page to master * commit '181563b524ce985b17096ccfe86082a82b24801d': Change "forget" to "clear" Add settings to clear localStorage items Authorize save_setting for global settings Add error and success alerts for classification banner settings Add configurable font color for classification banners Move classification banner code to own component Implement saving and loading classification banner Add classification banners Add classification banner settings page
I'm not done yet but take a look and see if you agree with my philosophy here. Basically I'm breaking up packet_config into individual keyword parsers which have top level class methods to parse them.