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

bz19198. don't generate possibly-broken embed code #16

Merged
merged 3 commits into from
Jul 2, 2012
Merged

Conversation

paulswartz
Copy link
Contributor

This also fixes some test failures.

We'll need to push out an 0.5.3 which has this fix as well, but since the existing development happened against develop we can review it for that, then merge into the release branch/master later on.

@melinath
Copy link
Contributor

These are all good commits, but only the first commit actually has anything to do with the issue at hand. The others should be in separate branches or directly on develop - or to put it another way, they don't need to be a part of this branch. In particular, 731cff3 is essentially a merge of the master branch changes to data load error catching, which we do need to do, but which shouldn't be done in this branch and should probably be done after or simultaneous to a merge of master into develop.

I don't think we need to push out 0.5.3. This isn't a major functionality issue or a security problem. It can wait until 0.6.

@paulswartz
Copy link
Contributor Author

"I don't think we need to push out 0.5.3. This isn't a major functionality issue or a security problem. It can wait until 0.6."

It's currently listed as a P1 bug for the 1.9 release of Miro Community. If it wasn't a "major issue" as least as judged by us via the bug tracker, I wouldn't be working on it at all.

@paulswartz
Copy link
Contributor Author

I pushed the other commits directly onto develop; they should disappear from the pull request shortly.

@melinath
Copy link
Contributor

You're absolutely right about this being a p1 1.9 bug. My bad. I'll have to think about this. The change really doesn't seem like it warrants a release inherently, and I don't want to make vidscraper releases every time that mc needs one. I'm leaning towards bumping that ticket back to 2.0.

@melinath
Copy link
Contributor

melinath commented Jul 2, 2012

Incidentally, the bug in question is bug 19198.

paulswartz and others added 3 commits July 1, 2012 22:12
This changes the Vidscraper behavior away from generating embed code.  Instead,
we'll use the URL given as a `file_url` so that users can generate their own
embed code in the frontend.
Removed _valid_enclosure_list function.
@melinath melinath merged commit 241dbbe into develop Jul 2, 2012
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.

2 participants