Skip to content

Conversation

@ellatrix
Copy link
Member

@ellatrix ellatrix commented Aug 2, 2018

Description

I don't think these are that useful to have because all of them have the same save functions and attributes as the embed block. They bloat PRs (such as #7890) that change the embed block, and it's a waste of time to update them all.

@ellatrix ellatrix added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Aug 2, 2018
@ellatrix ellatrix requested review from a team and ntwb August 2, 2018 14:23
@notnownikki
Copy link
Member

+100

Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Makes sense to me, I don't think they're useful tests looking at this diff now.

more-red-than-green

Copy link
Member

@notnownikki notnownikki left a comment

Choose a reason for hiding this comment

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

This is my favourite PR of the week.

@jasmussen
Copy link
Contributor

This thread: 🏆

@ellatrix
Copy link
Member Author

ellatrix commented Aug 2, 2018

Cool, that didn't need much convincing! :)

@ellatrix ellatrix merged commit 712cb1f into master Aug 2, 2018
@ellatrix ellatrix deleted the remove/oembed-fixtures branch August 2, 2018 17:05
@ntwb
Copy link
Member

ntwb commented Aug 3, 2018

Noice 💐

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants