Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "Nothing to do" log entry #2393

Merged
merged 2 commits into from
Dec 23, 2020

Conversation

reggieb
Copy link
Contributor

@reggieb reggieb commented Dec 11, 2019

#2392

Removes the "Nothing to do" log entry from the Webpacker::Compiler#compiler method.

Nothing to do should include logging!

@@ -27,7 +27,6 @@ def compile
record_compilation_digest
end
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else
else
logger.debug "Everything's up-to-date. Nothing to do"

@rossta
Copy link
Member

rossta commented Aug 28, 2020

@reggieb This feels like a good change to me. Want to accept the requested change so we can try to get this through?

@reggieb
Copy link
Contributor Author

reggieb commented Sep 1, 2020

@rossta I've reinstated the logger call with the level set to debug

@nick123pig
Copy link

This would be very helpful to merge in when able!

@david-mears-2
Copy link

@jakeNiemiec Tagging to bring this to your attention. You'd be very popular if you found time to re-review and merge!

@reggieb
Copy link
Contributor Author

reggieb commented Dec 23, 2020

Is there a reason why this has been closed without merging?

@gauravtiwari gauravtiwari reopened this Dec 23, 2020
@gauravtiwari gauravtiwari merged commit 716655f into rails:master Dec 23, 2020
@gauravtiwari
Copy link
Member

Sorry @reggieb

Was just trying to cleanup PRS and issues so we can look things afresh post 6.0 release.

@reggieb
Copy link
Contributor Author

reggieb commented Dec 23, 2020

@gauravtiwari No problem.

I'm really please this has finally been merged. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants