The Formatter::HTML should not inherit from the MessageBuilder - #1899
Conversation
* The Runtime creates the MessageBuilder to always output the envelope it creates from event to the event bus. Therefore a formatter only need to listen to the :envelope events on the event bus.
luke-hill
left a comment
There was a problem hiding this comment.
I think we need to ensure we're calling repository update if we no longer call super. Especially with the missing MessageHandler method. Which ensures that attachments are good
|
The HTML formatter does not have a repository, it just listens to the message stream and forwards the messages to the HTML-formatter from the html-formatter repo. It is the MessageBuilder instance in the Runtime (https://github.com/cucumber/cucumber-ruby/pull/1899/changes#diff-fc62aa05d38336fb22819196433b9acb4370acadd10dbc201a48ccbd5980df6dR178) that ensures that the full message stream is available on the event-bus and that the attachments are good. |
|
So will the Reason I ask is because we need the |
|
The |
luke-hill
left a comment
There was a problem hiding this comment.
Approving on the proviso that the on_envelope method for the MessageBuilder class is called as well as the on_envelope method for the HTML formatter (Sequentially and the MessageBuilder one needs calling first).
| # Define the MessageBuilder formatter - Required until all messages are generated at the source | ||
| # Skip when a user formatter already inherits from MessageBuilder (e.g. HTML) to avoid sending every event twice. | ||
| message_builder unless formatters.any? { |f| f.is_a?(Formatter::MessageBuilder) } | ||
| message_builder |
There was a problem hiding this comment.
This might need re-ordering and going above formatters
Description
Runtimecreates theMessageBuilderto always output the envelope it creates from event to the event bus. Therefore a formatter only need to listen to the:envelopeevents on the event bus.This change should have been part of #1865. Since it was missed there #1890 occurred. #1891 fixed the symptom of #1890 but failed to remove the inheritance fo the
Formatter::HTMLType of change
Please delete options that are not relevant.
Please add an entry to the relevant section of CHANGELOG.md as part of this pull request.
Checklist:
Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.
bundle exec rubocopreports no offenses