Get \Drupal\views\Plugin\views\display\Feed::attachTo in line with \Drupal\views\Plugin\views\display\DisplayPluginInterface::attachTo

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

This seems like something that will need a valid test case please

Thanks.

So what the original issue was talking about was calls to $this->view->destroy();, these are long gone. So the only destroy() remnant lives in \Drupal\views\Plugin\views\display\Feed::attachTo, but that destroys a clone that is build in the calling function. Weird, sure, but won't break anything.

\Drupal\views\Plugin\views\display\Feed::attachTo still talks about it being a clone, it is, but it's not relevant that it is. The parent interface doesn't name it a clone and the only other version of it in \Drupal\views\Plugin\views\display\Attachment::attachTo doesn't name it clone either.

So lets stop naming it a clone and stop treating it like one in \Drupal\views\Plugin\views\display\Feed::attachTo

This method has test coverage in \Drupal\Tests\views\Functional\Plugin\DisplayFeedTest::testFeedOutput so that covers any refactoring we do here.

Bleh, left something staged to test if we could remove the cloning from ViewsExecutable, but we can't (and would be out of scope here anyway)

So the patch I meant to post....

Seems like a cleanup that #23 pointed out has test coverage.

I'm triaging RTBC issues → .

I read the IS, comments and patch. All questions answered and I agree with the code changes. Assigning to myself to commit after a break.

Thanks for cleaning this up!

Committed 857aadc and pushed to 11.x.

Cheers

Automatically closed - issue fixed for 2 weeks with no activity.