#321 ✓ checked-in
Rick

Create: plugins/index.html

Reported by Rick | February 9th, 2011 @ 03:05 PM | in 0.4

2 columns

Left: 150px, UL with links to the all the plugin's unit test pages, click targets iframe

Right: iframe to display the plugin unit test pages

Comments and changes to this ticket

  • David Humphrey
  • danman

    danman March 4th, 2011 @ 10:00 AM

    • Assigned user set to “danman”
  • danman

    danman March 4th, 2011 @ 11:18 AM

    • State changed from “new” to “peer-review-requested”

    https://github.com/DanVentura/popcorn-js/tree/t321/plugins

    I'm no artist, but the code's there. index.html has frames, testDirectory.html has links to all the tests

  • annasob

    annasob March 4th, 2011 @ 06:30 PM

    • State changed from “peer-review-requested” to “review-needs-work”

    Hey dan,
    I think this needs to be more automated. We need something that will run all of the unit tests in each directory inside plugins. Something that just runs when you open index.html

  • annasob

    annasob March 6th, 2011 @ 10:19 PM

    Hey,

    Just realized that Rick's initial direction was followed. Dan do you have any ideas on how to further automate this?

  • danman

    danman March 6th, 2011 @ 10:38 PM

    • State changed from “review-needs-work” to “peer-review-requested”

    https://github.com/DanVentura/popcorn-js/tree/t321/plugins

    Took me a while but finally got the tests to be automated. Included an option to run all of the tests in succession or stay on a test of choice.

  • Rick

    Rick March 7th, 2011 @ 09:32 AM

    • State changed from “peer-review-requested” to “under-review”
  • annasob

    annasob March 7th, 2011 @ 07:53 PM

    This looks good except it has some changes to popcorn.js let me know if you need help quickly recreating the branch. This is as automated as its gonna get w/o doing stuff in make. Good work. Rick can SR but the branch will most likely need to be updated before that.

    PR+

  • danman

    danman March 7th, 2011 @ 09:28 PM

    Forgot about that. Replaced the popcorn.js code with the commit from before I changed anything.

    Same branch as above.

  • Rick

    Rick March 8th, 2011 @ 10:08 AM

    • State changed from “under-review” to “review-needs-work”

    This is a really creative and helpful solution.

    A few things... I'm seeing changes to the subtitles and wikipedia plugins that cause a merge conflict. Are there changes to those files that the plugins/index.html file requires?

    Also, I'm going to create a list of things we need to think about to improve the system overall, but those changes will be a round 2 and exist on another ticket.

  • danman

    danman March 8th, 2011 @ 12:48 PM

    My bad. I shouldn't branch from my other branches... Removed all my changes to those plugins:

    https://github.com/DanVentura/popcorn-js/tree/0d0a90d0fc0d2fc08b763...

    Only thing to keep in subtitle/ is the unit tests.

  • annasob

    annasob March 8th, 2011 @ 02:32 PM

    • State changed from “review-needs-work” to “super-review-requested”
    • Assigned user changed from “danman” to “Rick”

    Rick I suggest this round 2 can be done for both this and #320. Putting back to SR, I PR+ this and looks like dan fixed the branch.

  • Rick

    Rick March 8th, 2011 @ 02:35 PM

    • State changed from “super-review-requested” to “under-review”

    Agreed.

  • Rick

    Rick March 8th, 2011 @ 02:38 PM

    Normally I'd say we need to avoid all the inline event handlers, but I'll add that to that 0.5 upgrade list.

  • Rick

    Rick March 8th, 2011 @ 02:44 PM

    • State changed from “under-review” to “review-looks-good”

    Good to go.

  • Rick

    Rick March 8th, 2011 @ 02:44 PM

    • Assigned user changed from “Rick” to “annasob”
  • annasob

    annasob March 8th, 2011 @ 04:01 PM

    • State changed from “review-looks-good” to “review-needs-work”
    • Assigned user changed from “annasob” to “danman”

    Dan I have some issues with this. The branch seems to have a lot of "removed my fix" stuff. However, my main issue is the index.html file missing plugins. Here are the fixes that need to happen:
    - ensure all the plugins are present (code and lastfm missing for sure) - ensure that all testDur are accurate, sometimes the test skips to the next test before you see the resuts - change i <10 to i < testUrl.length;

    All these changes should happen when you fix the branch. To do so make a copy of the index.html file. Then create a fresh branch:

    git checkout master
    git branch 321b
    git checkout 321b
    git pull git://github.com/annasob/popcorn-js.git 0.4 //update with my 0.4
    git add index.html
    

    then make the changes then make a new commit.

    Sorry that I didn't catch this earlier. Hopefully git will not give you any trouble. Put back to SR when done

  • danman

    danman March 8th, 2011 @ 06:18 PM

    • State changed from “review-needs-work” to “super-review-requested”

    https://github.com/DanVentura/popcorn-js/tree/t321b/plugins

    • created new branch (t321), updated to 0.4 and added index.html

    Commit

    • added subtitle tests
    • added a line to popcorn.subtile.js. It now sets the subtitle div to "subtitlediv". Without this line, the subtitle unit test creates a ridiculous amount of errors

    Commit

    • added plugins to index.html: mustache, code, lastfm
    • redid test durations
    • fixed a bug in clearEvents(). It never really cleared all timeouts

    Commit

    Whew

  • Scott Downe

    Scott Downe March 9th, 2011 @ 12:17 PM

    • State changed from “super-review-requested” to “review-looks-good”

    I am warming up to this as I play with it but.

    I think there may be some bugs, but as an initial push, this is a very good step forward. We can fix bugs later, what I am concerned about is breaking existing code, and I don't see that happening.

    I agree with Rick that we can tweak this more in 0.5.

    Also, to simplify things, I recommend just doing this ticket (#321) and #505 in one stage.

  • annasob

    annasob March 9th, 2011 @ 07:55 PM

    • State changed from “review-looks-good” to “staged”

    All the changes I requested were made.

    Staged in annasob/popcorn-js commit

  • annasob

    annasob March 21st, 2011 @ 02:47 PM

    • State changed from “staged” to “checked-in”
  • annasob

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Tags

Referenced by

Pages