Comment up Chef::Client and privatize/deprecate unused things #3392
Conversation
jkeiser
changed the title
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| require 'chef/client/notification_registry' |
There was a problem hiding this comment.
Eep! Old thing, empty file.
There was a problem hiding this comment.
Err, no, that is actually something--there was so much stuff in there about notifications that it seemed useful to separate those methods out, group them--thus the module.
There was a problem hiding this comment.
Yeah but in this line aren't you requiring the file you're in?
|
@lamont-granquist based on our talk last week, the main question was whether we should deprecate the attr_writers in question. I have deprecated If that's all OK, I'mma merge. |
|
tests are pissed, probably needs a rebase? |
|
@chef/client-core this needs review |
|
@lamont-granquist the test failures were a bad rebase, actually, I've fixed it up. Thanks :) |
|
|
||
| class Chef | ||
| class Client | ||
| module NotificationRegistry |
There was a problem hiding this comment.
If we're pulling this out, I'd rather it just be a class and then you delegate to it (using Forwardable if you like).
There was a problem hiding this comment.
I can just put it back in--only reason to put it out in another file was to be able to read it in chunks a bit. I don't think it'll be reused.
|
👍 |
No description provided.