revise existing processes/tools to publish all supported brands using 1 release branch

[pixelzoom]

In #587, we agree on interim naming conventions for release branches, where phet and phet-io brands each have their own branch. That convention relies on manual synchronization and relatively complex naming conventions.

11 days after discussing it, it's already not working. @jonathanolson has run into problems with maintenance releases. And I've spent 5x as long as I should have trying to do a simple maintenance release (see phetsims/plinko-probability#102 (comment)). In all cases it's because there's confusion about what we decided, or what we decided is not being followed.

For the longterm, we agreed that having parallel release branches is bad, and all supported brands should be published out of 1 release branch, having the "major.minor" branch naming convention (e.g. "1.1"). Given the slow progress on discussion of general PhET-iO issues (see https://github.com/phetsims/phet-io/issues/1137), revamping build tools/processes is unlikely to happen for a long long time. So imo, we need to figure out how to published with 1 release branch now, with relatively minor changes to existing tools/processes.

This issue will track the discussion and decisions.

[pixelzoom]

One of the main issues is that different brands have different dependencies. For example, brand=phet-io has additional repositories related to PhET-iO. Could we add a brand suffix to dependencies.json, for example dependencies-phet-io.json? The absence of the suffix indicates brand=phet.

[pixelzoom]

What are the other problems with using 1 branch for releasing multiple brands?

[zepumph]

If we are using one branch, then maintenance release numbers between brands are more tightly meshed. If I need to make a small fix in a phet-io repo for a maintenance release, then I will pump the maintenance version without publishing a phet brand version. I'm sure there are cases where this applies the other way around as well. If we proceed like this it seems like we need to either (1) be alright with skips in version numbers for a single brand (1.1.0 1.1.1-phetio 1.1.2 ) or (2) come up with a way of updating the package.json to different versions depending on the brand.

It does seem simpler if there was a solution where you always published both, so a branch's version is always one thing, but in the current state of the build tools, it would put more strain on QA and the deployer, who would always have to manually build and deploy both.

[jonathanolson]

it would put more strain on QA and the deployer, who would always have to manually build and deploy both.

Presumably for the automated maintenance process, it would just deploy both. For QA, the amount of testing for a maintenance presumably ranges from 30 seconds per sim to 5 minutes per sim (testing very specific things), and for most maintenance releases testing two brands would probably consist testing mainly in one brand, and just opening another brand to make sure it works.

[samreid]

It does seem like moving to a single branch will be best. This makes the most sense for sims like "molarity" where both brands were tested/published at the same time. Probably doesn't make sense to do retroactively for pre-existing sims (but this is the case for Plinko Probability so I'm not certain).

[jonathanolson]

Probably doesn't make sense to do retroactively for pre-existing sims

If we don't handle it retroactively, it means we need extra logic in the build/maintenance process presumably?

[samreid]

I like how accessibility is "built in" to the main version and doesn't require a special build or query parameters. It may not be possible to do exactly this with phet-io (but from an engineering standpoint it would be best if we could), but perhaps we should think of the the PhET-website sim as a subset of the PhET-iO simulation. That is, we could move to a "PhET-iO First" development methodology, then "carve out" the PhET brand sim from the PhET-iO one.

[samreid]

We decided to wait until October/November to work on this as part of the chipper 2.0 issue, see #596

[samreid]

If we are automating everything as much as possible, perhaps we would have a machine-readable database that indicates whether PhET-iO was tested for a sim, then the build process would automatically build the tested versions.

[zepumph]

One question that @ariel-phet brought up is confusion around testing for a particular release. If we are building in one branch, but only test the phet version, how do we handle that?

Is our process that we just test everything always. Default is to test the phet-io version every time, but perhaps it will take some time to get there.

@jbphet thinks that in the new chipper:2.0 we should have the ability to say one brand was fully tested, but not the other.

@ariel-phet: we could keep a master qa spreadsheet of the testing that has been done to every sim/version/brand.

@samreid: we should try to gear towards as automated as possible. Perhaps like a machine readable database that has all of the testing done on a sim/version/brand.

We want to get @kathy-phet's opinion on this.

[samreid]

Dependencies.json should contain the union of phet dependencies and phet-io dependencies (or generally for all brands).

[samreid]

We decided it is OK for dependencies.json to contain information for multiple brands since that only determines what is checked out during checkout-shas, not what is a dependency for a build for a particular brand.

[jonathanolson]

The "allow the latest phet and phet-io builds to be on different versions" (so we don't always have to push out to all brands when we make a fix) seems to have a lot of issues if we try to keep 1 branch for those 2 versions.

As an example, say Molecule Shapes 1.2.0 gets released (has a branch 1.2 in molecule-shapes), and then we apply a phet-io fix (updating only phet-io), thus we would have at one time:

• 'phet' brand version 1.2.0
• 'phet-io' brand version 1.2.1

Then we notice something that needs a maintenance release (say our sims cause the new samsung phone to explode). Presumably the maintenance release would create version 1.2.2, but it would force through untested phet-brand patches (the changes made for 1.2.1).

If we really want to have phet and phet-io brands be on different versions, it seems that we really need to have different branches that can have maintenance releases applied separately. If we don't, maintenance releases will have to:

1. Push through untested changes that were only deployed to one brand
2. Will force more testing for important maintenance releases (maintenance release process will need to manually identify all of the times we just published to one brand, and will force QA to test those changes on the other brand).

@ariel-phet and developers, is (1) or (2) acceptable, or should we have different release branches for different brands? (or a third option?)

[zepumph]

What if the maintenance release qa process involves testing the change for all brands. So even if it is phet maintenance release patch, you test the phet-io stuff too. Ideally that is what we want in master also, because we don't want changes to break any brand (I phet and phet-io fuzz before committing any common code).

I don't see this as a large cost. Aren't maintenance releases mainly just spot checks anyways right?

[jonathanolson]

I'd be happy to revisit the "test and deploy both brands for any change" approach.

[zepumph]

"test and deploy both brands for any change"

Why would you want to deploy a brand no matter what, even if you don't need to? I feel like that is unneeded and an extra cost if no one will be using the other brand. With the quoted approach you would also run into edge cases that would lead to discrepancies. How would you handle a situation where a cherry-pick only touches phet-io code. Would you create a maintenance release for phet brand that has the exact same code in it as the previous release?

What if we centralized around and approach that made sure that release branches were always safe to deploy any brand. This means that cherry-picking a commit into that branch should be tested on all brands that could be released from that branch.

The next question to solve then becomes how to "test" all brands when you cherry-pick a commit into a release branch. The way that we would do that now is to build all brands and then deploy them to the dev server for testing. Perhaps we could improve that process when we know we are just testing a brand to "make sure it isn't broken from a cherry-pick" rather than because we want to deploy it in a maintenance release. What if phet-test could support release branches shas for easier testing?

[jonathanolson]

What if we centralized around and approach that made sure that release branches were always safe to deploy any brand. This means that cherry-picking a commit into that branch should be tested on all brands that could be released from that branch.

Sounds fine to me (personally), but I'm curious what @ariel-phet thinks. It makes sense that if a commit touches phet-io, it won't affect the phet-brand release and should be totally safe (as you noted above).

The next question to solve then becomes how to "test" all brands when you cherry-pick a commit into a release branch.

Usually for maintenance releases (particularly multi-sim ones), I'll provide instructions of what needs to be tested (e.g. phetsims/qa#72). We'd probably deploy an automated RC for both brands, and would add instructions about what to test.

[jbphet]

+1 for the idea that RCs are always built and tested for all brands, but deployments are only done for the brands that need the release.

[zepumph]

We'd probably deploy an automated RC for both brands, and would add instructions about what to test.

"both" meaning 2 now, but supporting built/test for any brand necessary in the future?

I like that idea. I will say that we should be careful about what we deem appropriate as a phet-io test if cherry-picking something for phet brand. It may not always be as simple as to run the stand alone simulation in phet-io brand (the first option for the index wrapper). I can imagine a case where a cherry-pick would run seemlessly in phet brand, or phet-io standalone (not in a wrapper), but would break the state or mirror-inputs wrappers. I guess I would simply recommend knowledge of all brands when addressing instructions of what needs to be tested.

[pixelzoom]

Self assigning so that I remember to review comments on this issue that occurred since 11/17/17.

[samreid]

The following hypothetical situation seems reasonable to me:

1. Publish 1.1.0 of Faraday's Law, both the PhET and PhET-iO brands, and create a single 1.1 maintenance branch.
2. Receive bug reports that there is a problem with the PhET-iO version (that doesn't impact the PhET branded version).
3. Apply fixes in the 1.1 branch.
4. Test the PhET-iO branded version and redeploy it.

Later when we are ready to publish a maintenance release of the PhET-branded version, we can thoroughly test it. This means it will not be possible to do automated batch maintenance on it, since it will need manual testing for the PhET-iO related changes. How do we track this? I'm not sure.

[jonathanolson]

This means it will not be possible to do automated batch maintenance on it, since it will need manual testing for the PhET-iO related changes. How do we track this? I'm not sure.

That sounds like a problem I'd like to avoid. Not being able to do batch maintenance releases sounds unacceptable.

However, the batch maintenance process could identify when maintenance versions differ between phet and phet-io, and could note that those RCs need additional testing. Then QA would need to look up what changes have been applied (so they could test). Thoughts?

[zepumph]

The above from @samreid seems less than optimal.

4. Test the PhET-iO branded version and redeploy it.

We already basically fully tested the PhET version in this step, why not document it as such/ take the steps to assure that the PhET brand is good to go as well?

[samreid]

The reason I suggested a hypothetical example in #592 (comment) of the PhET-iO version needing changes (but not the PhET version) and not vice-versa is that all PhET-brand bugs impact PhET-iO versions as well. That is, if it is important enough for us to do a maintenance for a bug for a PhET brand sim, then we should probably also maintenance release the PhET-iO sim as well.

Hence, maybe we are best off to test/deploy both brands together.

[jbphet]

@zepumph said:

We already basically fully tested the PhET version in this step, why not document it as such/ take the steps to assure that the PhET brand is good to go as well?

Two points on this:

1. The additional testing is an additional requirement on the QA process, and will grow as the number of supported brands grows. I haven't seen anything from @phet-steele on this topic, and he should probably chime in, the basic question being "How much harder would it be to have to QA team verify all brands for every release instead of just the needed brand or brands?"
2. Even if we test all brands, we shouldn't deploy them, since we don't want to trigger unnecessary updates for sim users, in the app, and probably in future phet-io applications.

From a purity standpoint, I think verifying all brands each time a maintenance release occurs is the way to go, as it solves problems like knowing whether or not a bulk maintenance release can be done. It's a matter of whether it is practical to have this policy in place, which will take input from @ariel-phet and @phet-steele.

[zepumph]

@samreid said:

Hence, maybe we are best off to test/deploy both brands together.

@jbphet said

2. Even if we test all brands, we shouldn't deploy them, since we don't want to trigger unnecessary updates for sim users, in the app, and probably in future phet-io applications.

I agree with both of these. I don't think we should go with the "always deploy all brands" approach, but instead have heuristics.

1. If there is a PhET brand Maintenance release, there should probably a phet-io one because @samreid is right, this overlaps directly for things like security/scenery fixes and what not.
2. A PhET-iO specific fix definitely doesn't need to deploy for phet brand.
3. I think that leaving developer discretion in the mix is important, especially as we move through alpha/beta with phet-io. We are going to want to do special cases that are different from the "ideal" build environment we want to see long term.

[pixelzoom]

My recommendation is to always publish all brands. Yes, it increases testing costs, and will bother users with potentially irrelevant updates. But it simplifies build tools and processes, which involve development resources that are much more expensive.

[jonathanolson]

As an addendum, I'd like to recommend that dev/rc deployments should build/deploy both phet and phet-io brands (if supported by the sim) with the same SHAs and version number. e.g. if I deploy charges-and-fields dev, it would push phet and phet-io.

It doesn't really work to deploy an RC first for phet-brand (which bumps shas/dependencies/etc.) and then deploy an RC second for phet-io, and they would get different versions (rc.1-phet, rc.2-phetio).

As part of this proposal, I'd prefer that the information in chipper/data/phet-io (which sims support phet-io) be moved into the package.json (and we can generate chipper/data/phet-io whenever it changes). I believe I've proposed in the past that we do this for everything but active-repos (as we would need some list in order to generate the others).

[jonathanolson]

Presumably under the phet object in package.json, we'd have a supportedBrands?

[pixelzoom]

+1 for dev/rc deployments should build/deploy all supported brands. (Proposal was for "both phet and phet-io brands" but let's not do anything in the build tools that precludes future brands.)

[jonathanolson]

I was thinking we wouldn’t deploy adapted-from-phet specifically, but if we have another production brand it would be added.

[pixelzoom]

Just don't put "adapted-from-phet" in supportedBrands and it won't be built, right?

[jonathanolson]

That works well, sounds good.

[pixelzoom]

12/4/17 dev meeting:

Yes, let's add supportedBrands, required field in package.json. E.g.:

{
  ...
  "phet": {
    ...
    "supportedBrands": [ "phet" ]
  }
}

• [ "phet" ] will be the default in the (simula-rasa) template.
• brands=* will do all brands listed in supportedBrands.
• If you try to build a brand that's not in supportedBrands, build process will fail.
• Add utility for re-generating chipper/data/ files based on all package.json.
• Handle use case of 3rd party trying to build without access to phet-io repository.

[jonathanolson]

If you try to build a brand that's not in supportedBrands, build process will fail.

It's kind of nice to specify a default set of brands that would be built when they are available. (for instance, if the default is phet,phet-io in your build-local.json, I don't want all uninstrumented sims failing out since I tried to "built phet-io").

Is that acceptable (and instead we can fail out dev/rc/production deploys if there is an unsupported brand, since you manually specify them in that case)?

Otherwise, this work is basically complete (will need to get 3rd party build support however).

[jonathanolson]

Added 3rd party build support (lack of any phet-io dependencies will fail out phet-io build or ANY dev/rc/production deploy process).

[pixelzoom]

@jonathanolson said:

It's kind of nice to specify a default set of brands that would be built when they are available. (for instance, if the default is phet,phet-io in your build-local.json, I don't want all uninstrumented sims failing out since I tried to "built phet-io").

I think that's fine if specified in build-local.json -- semantics in build.json would essentially be "build as many of these brands as are supported". But if --brand is explicitly on the command line, then I think it should fail if one or more of the specified brands is unsupported.

[jonathanolson]

If --brand is supplied, it will fail out if one is unsupported. If --brand is not supplied, it checks build-local.json's brands (an array of strings), and will build all supported brands from that list (so for an uninstrumented simulation, it will not build phet-io and that's fine). If build-local.json doesn't have brands, it will default to [ 'adapted-from-phet' ].

[jonathanolson]

Double-checked. Removing build-local.json and checkout out a limited subset of repos (the ones in a sim's README.md) results in grunt building the adapted-from-phet brand successfully.

Given that the main work described in here is implemented in chipper's 2.0 branch (and the deployment commands for handling one branch would be handled in #616), can this be closed? @mattpen is there anything else to do here?

[mattpen]

This has been working nicely since the merge. Closing.