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

Publish a Legends of Learning Prototype #162

Open
samreid opened this issue May 28, 2017 · 56 comments
Open

Publish a Legends of Learning Prototype #162

samreid opened this issue May 28, 2017 · 56 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented May 28, 2017

As part of phetsims/joist#423 I am building a Legends of Learning prototype of Build an Atom. I want to use safe SHAs to limit the amount of necessary testing for the QA step. The latest published version is 1.2.5. However, the 1.2 branch dependencies.json says 1.2.3. @jbphet can you please help me understand this discrepancy?

@samreid
Copy link
Member Author

samreid commented May 28, 2017

I'm not seeing 1.2.5 mentioned in this repo or tasks repo issues. But its shas do differ from 1.2.3. I guess I should use 1.2.5 shas?

@samreid
Copy link
Member Author

samreid commented May 28, 2017

My apologies! I was out of date in the branch. It looks like this bumped to 1.2.5 in a maintenance release with this commit message:
Bumping version to 1.2.5 for phetsims/phet-android-app#16,phetsims/phetcommon#37

@samreid
Copy link
Member Author

samreid commented May 28, 2017

The shas to cherry pick are: phetsims/chipper@1cb1805

and
phetsims/joist@bb8957a

So I guess I'll need branches for these.

@samreid
Copy link
Member Author

samreid commented May 28, 2017

It looks like Build an Atom 1.2.5 chipper predates query string machine, so that won't work as a cherry-pick. Build an atom 1.2.5 joist is not using Emitters, so this cherry pick won't work either.

Main choices from here:
(1) build custom "legacy" patches for this sim and possibly others
(2) fully retest build an atom

Tagging @ariel-phet and @kathy-phet so they are aware what's happening here.

@samreid
Copy link
Member Author

samreid commented May 29, 2017

I pushed a custom patch in the above commit. @jessegreenberg can you please review the change?

@samreid
Copy link
Member Author

samreid commented May 29, 2017

I made this change in the maintenance branch, but not sure if that's the best spot--I'm concerned this would leak into production if another maintenance release is done.

I published an RC here:
http://www.colorado.edu/physics/phet/dev/html/build-an-atom/1.2.6-rc.1/build-an-atom_en.html?legendsOfLearning

It tests OK in the harness in Chrome.

@kathy-phet and @ariel-phet it would be good to get your feedback for next steps. Some possibilities:

  1. Deliver the RC to Legends of Learning. Since I used the safe SHAs, this should be identical to the website version with the addition of the legends of learning hooks. Is it OK if they get an "RC" version (shows in the about dialog)? If we want to bump to a standard production (non-rc) version, should it also be published on our website?
  2. Should we do any QA testing? For instance, testing on different platforms to make sure the Legends of Learning code doesn't disrupt anything? It doesn't look too risky but I haven't tested broadly.

@ariel-phet
Copy link

ariel-phet commented May 30, 2017

@samreid I am not concerned about "RC" showing up in the about dialog, since almost no user will look there, and even fewer will know what RC means

However, if we are going to be regularly publishing to legends of learning, this code should be in production and well tested (since maintenance releases are sure to occur, as they happen regularly). If we are going to make this code standard it should have QA done when put in production.

If this is just for a prototype and needs quick attention, the QA team is currently back logged (which should be improved in the next week or so as team members start summer schedule.

Would it be OK to deliver the prototype, make sure things are working on LoL end, make any necessary tweaks, and then do a full QA test to bring to production?

@ariel-phet ariel-phet removed their assignment May 30, 2017
@samreid
Copy link
Member Author

samreid commented Jun 1, 2017

@ariel-phet @kathy-phet and I should discuss this on Thursday

@samreid
Copy link
Member Author

samreid commented Jun 1, 2017

Also @jbphet addressed #161 so if we cherry-pick that fix we could get the timer to pause on the game screen. Not sure if that is important for first prototype in their platform.

samreid added a commit that referenced this issue Jun 2, 2017
@samreid
Copy link
Member Author

samreid commented Jun 2, 2017

@jessegreenberg and I tried to cherry pick the change but the branches diverged too much so it couldn't auto-cherry-pick. We ended up applying the same changes manually. It seemed OK in testing.

samreid added a commit that referenced this issue Jun 2, 2017
@samreid
Copy link
Member Author

samreid commented Jun 2, 2017

New RC is here:
http://www.colorado.edu/physics/phet/dev/html/build-an-atom/1.2.6-rc.2/build-an-atom_en.html?legendsOfLearning

I'll create a tasks issue for testing it.

@samreid
Copy link
Member Author

samreid commented Jun 12, 2017

One problem was discovered during RC testing:
phetsims/scenery#619

@jonathanolson and I solved it by adding interrupt methods to input listeners. However, neither of the commits is cherry-pickable for the build an atom SHAs.

@samreid
Copy link
Member Author

samreid commented Jun 12, 2017

@jessegreenberg are you available to create a new Build an Atom RC that uses the same SHAs as http://www.colorado.edu/physics/phet/dev/html/build-an-atom/1.2.6-rc.2/build-an-atom_en.html?legendsOfLearning with the two new commits (probably manually patched in) from scenery and joist in phetsims/scenery#619 ?

@jessegreenberg
Copy link
Contributor

@samreid sure, I will prepare the next RC today.

@samreid
Copy link
Member Author

samreid commented Jun 12, 2017

Thanks and good luck! Let me know how tricky it is to patch in those commits, we may be doing it for 11 more legacy joist and scenery shas.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 13, 2017

Merging was pretty easy, all things considered. There were merge conflicts in Sim.js and SimpleDragHandler.js, but they were not too difficult to work through (things like renaming to self and such). Node.js was missing implementations for interruptInput and interruptSubtreeInput, I had to include those, but that was straight forward.

However, I am still seeing a bug in the legends of learning test harness, particles freeze after pressing pause during dragging
capture

To see this in the harness,

  • Press "Pause in 5 seconds"
  • Drag two particles around until paused
  • Press resume

Particles will continue to move until they hit this position. This is a problem for both master and 1.2 branches. Does not happen every time.

@jessegreenberg
Copy link
Contributor

Not sure if this is a sim specific bug or a problem with changes in phetsims/scenery#619. This never happens when changing screens, only when using Legends of Learning pause feature.

@samreid
Copy link
Member Author

samreid commented Jun 15, 2017

@jonathanolson and @jbphet will look into this by tomorrow, possibly with help from @jessegreenberg

@ariel-phet ariel-phet removed their assignment Jun 15, 2017
@jessegreenberg
Copy link
Contributor

Properly cherry-picking commits from Scenery to a branch 300+ commits old (and whatever Joist is) sounds risky.

It sounds like we all agree that maintenance releases and patching is too risky for these releases because the SHAs are too old.

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 21, 2017

@samreid This issue is labeled for developer meeting. Can you summarize what feedback you're looking for?

@samreid
Copy link
Member Author

samreid commented Jun 21, 2017

From the last meeting:

@jonathanolson and @jbphet will look into this by tomorrow, possibly with help from @jessegreenberg

It would be good to get a status update whether in issue comments or in developer meeting. I'll leave it marked for developer meeting in case it doesn't get discussed elsewhere.

@jessegreenberg
Copy link
Contributor

On 6/15/20, @jbphet, @jonathanolson and I followed the steps listed in #162 (comment) on @jbphet's machine on master and observed the particles getting stuck after a couple attempts.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jun 22, 2017

And there are some updates in the investigation of phetsims/scenery#619

@jessegreenberg
Copy link
Contributor

It sounds like @jonathanolson is confident in the fix for #phetsims/scenery#619. I will create an issue in QA to verify that things are looking good before proceeding with the next RC.

@samreid
Copy link
Member Author

samreid commented Jul 12, 2017

If I'm reading it correctly, the solution is working well on all platforms except Edge, which we decided is OK for now. So it seems the next step is to publish a Legends of Learning prototype--will this need another QA cycle? Or is there some safe SHA we should start with?

@jessegreenberg
Copy link
Contributor

Verification for phetsims/scenery#619 was done without a built version, I don't think there are safe SHAs to grab. When ready I think this will require a new RC task.

@samreid
Copy link
Member Author

samreid commented Jul 13, 2017

We agreed @jessegreenberg would build an RC this week.

@kathy-phet
Copy link

@jessegreenberg - Just FYI ... Josh emailed today was asking if this had been solved. Hopefully the RC testing will go well!

@jessegreenberg
Copy link
Contributor

Sounds good @kathy-phet, RC test was prepared in the above issue.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jul 28, 2017

1.6.0 was deployed on 7/27/17. The next step is to create a square icon with minimum dimensions of (400x400) for this sim to be seen on the Legends of Learning page.

We reached out to Legends of Learning to determine if we need to have the icon completed before publishing the sim to their platform or if we can use a "work in progress" icon for now and update later so we can continue going through the steps to add a sim to their site.

@jessegreenberg
Copy link
Contributor

Legends of Learning made it clear that we can easily update the icon, so we went through the publication steps for Legends of Learning. We discovered a few issues during the process:

  • Legends of Learning currently doesn't allow us to specify query parameters for the submission (required to communicate with the platform and run as a single screen).
  • Legends of Learning requires a file in the submission to be named index.html to run on their platform.

We met with Legends of Learning on 8/4/17 to discuss these issues, they will work on allowing us to specify query parameters during the submission process and possibly make it so we don't need to rename the submitted file.

On hold until we hear back from them.

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

No branches or pull requests

7 participants