Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @matthewd (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

I think this one make sense to be info since you want to know when the broadcast is being done. Maybe we should only include the message in the debug mode?

r? @dhh

cc @matthewd

I would also be happy to remove the #{message} part. If we remove that it would be a lot more readable for me.

Like this:

server.logger.info "[ActionCable] Broadcasting to #{broadcasting}"

I'd stick with info and just truncate the message part so it doesn't go crazy on a big HTML send. Maybe just do the first 300 chars or so.

Hi @dhh, I changed the commit as you proposed, I also added three dots (...) so we have a clue this could be shortened. My log now looks like this (which is much better):

PrinterListChannel transmitting "<div class=\"mod-printers-list\" data-id=\"printer-list\">\n  <table class=\"list-view\">\n    <thead>\n      <tr>\n        <th class=\"lead\">\n          Printers\n        </th>\n        <th>\n          Job\n        </th>\n        <th>\n          Time remaining\n        </th>\n        <th colspan=\... (via streamed from PrinterListChannel)
JobListChannel transmitting "<table class=\"list-view mod-jobs-list\" data-id=\"job-list\">\n  <thead>\n    <tr>\n      <th class=\"lead\">\n        Jobs\n      </th>\n      <th>\n        Printer(s)\n      </th>\n      <th>\n        Amount of prints\n      </th>\n      <th>\n        Created at\n      </th>\n    </tr>\n  </thead... (via streamed from JobListChannel)

I would love to hear if this is ready to merge, or if it needs a little more work.

This would give us three dots even when you're not truncating. You can use String#truncate from Active Support to do this.

I changed it, now it uses ActiveSupport's truncate method. Is this ready to merge?

You don't actually need to specify the omission part. Three dots is the default. So it can just be truncate(300).

Whoops, sorry! Changed!

👌