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

Allow to use custom reporters #2

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Allow to use custom reporters #2

merged 2 commits into from
Oct 13, 2023

Conversation

fatkodima
Copy link
Owner

Fixes #1.

It should be used like

class MyReporter
  def report(title, created_records)
    # do actual reporting
  end
end

ColumnsTrace.reporter = MyReporter.new

cc @andriy-baran Wdyt on the implementation?

@andriy-baran
Copy link

Looks great. Only thing to improve is documentation
Something like this:

class MyReporter
  def report(title, created_records)
    title # => 'controller#action'
    created_records # => [<#ColumnsTrace::CreatedRecord>]
    created_records.each do |record|
      record.model # class of AR model
      record.accessed_fields  # array of fields
      record.unused_fields # array
      record.backtrace # string
      record.record # AR model instance
    end
  end
end


def unused_fields
# We need to store this into local variable, because `record.attributes`
# will access all attributes.

Choose a reason for hiding this comment

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

Do we need to call accessed_fields before unused_fields to use it right, or order does not matter?

Copy link
Owner Author

@fatkodima fatkodima Oct 13, 2023

Choose a reason for hiding this comment

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

It should not matter, I pushed a small fix for this to actually be the case.

@fatkodima
Copy link
Owner Author

Thanks for review! Improved docs.

@fatkodima fatkodima merged commit 40bc8ff into master Oct 13, 2023
12 checks passed
@fatkodima fatkodima deleted the reporters branch October 13, 2023 11:04
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.

[Feature request / Question] Alternative way to notify end users about issues
2 participants