use Dist::Zilla::Role::FileWatcher #18
Conversation
|
|
||
| # If the new content is actually different, recalculate | ||
| # the content based on the updates. | ||
| if ($watched_file->content ne $self->_last_source_content) |
There was a problem hiding this comment.
Is this test actually necessary? Doesn't File::ChangeNotification already do this check?
There was a problem hiding this comment.
Probably not. It was there before so I left it in.
I think the point of this was to address a concern you'd raised initially - that if a munger changed the content, and then it was later changed back again to the first form, we wouldn't want to re-munge the file in that case as it is just a waste of resources. But there's no other harm that I can see in re-munging. Your call.
There was a problem hiding this comment.
Well, I think I previously assumed that File::ChangeNotification would trigger every time the file was written to, even if the contents hadn't changed. As far as I can tell, the test here is completely redundant with the same check in File::ChangeNotification (assuming there are no hash collisiions).
karenetheridge
force-pushed
the
topic/filewatcher
branch
from
ba3eefd
to
8e4318f
Compare
|
I force-pushed an improvement. |
use Dist::Zilla::Role::FileWatcher
This makes your code simpler, so you don't have to mess around with applying the role directly.
I think your pod_for_source_file code from https://github.com/DarwinAwardWinner/Dist-Zilla-Role-DistFileReader/blob/master/lib/Dist/Zilla/Role/PerlModuleReader.pm#L31 would be best added directly to Dist::Zilla::Role::PPI (where you can cache the PPI document, being even more efficient).