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

version id scheme #560

Closed
pixelzoom opened this issue Apr 11, 2017 · 68 comments
Closed

version id scheme #560

pixelzoom opened this issue Apr 11, 2017 · 68 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 11, 2017

The version id for PhET sims has (imo) gotten a bit confused with the introduction of PhET-iO. The 'phetio' brand identifier has been tacked on in a non-general way, and I have yet to see anyone provide a coherent/comprehensive description of the version id's syntax and semantics (getVersionForBrand.js included).

Current version id scheme

Here is the current format for public, dev and RC versions respectively, when brand is 'phet':

1.1.0
1.1.0-dev.1
1.1.0-rc.1

So far, so good. Now here is the current format when brand is 'phetio':

1.1.0-phetio
1.1.0-phetiodev.1
1.1.0-phetiorc.1

In the public version case, the build process adds a '-phetio' prefix. In the dev and RC cases, the build process reads the version id from package.json and replaces 'dev' and 'rc' with 'phetiodev' and 'phetiorc' respectively.

A few problems with this scheme:

(1) It modifies what the developer puts in package.json.
(2) It modifies test and public versions differently.
(3) It combines the "test type" and "brand" info, making it more difficult to parse.
(4) It is not general enough to support future test types or brands.
(5) It's difficult to write a "general form" description.

I'm proposing that we revised the version id scheme, as described below.

Proposed version id scheme

The version id scheme will consists of 3 chunks:

number[-test][-brand]

where:

number describes the version number
• test describes the test (absence indicates a public version that has been QA tested)
brand describes the brand (absence indicates 'phet' brand)
• square brackets indicate an optional chunk
• hypen is the separator between chunks

Expanding the chunks, we have:

major.minor.maintenance[-testType.testNumber][-brandName]

where:

• major = major number, sequential integer
• minor = minor number, sequential integer, resets to 0 when major is changed
• maintenance = maintenance number, sequential integer, resets to 0 when minor is changed
• typeType = the type of test, currently 'dev' or 'rc'
• testNumber = sequential integer indicating the test number
• brandName = the brand name, e.g. 'phetio'. Absence indicates 'phet' brand.

The developer specifies the number[-test] portion of the version identifier in package.json. The build process add the [-brandName] portion, if applicable, to each supported brand that is built.

Examples:

1.5.0 = public version of 'phet' brand
1.5.0-phetio = public version of 'phetio' brand
1.5.0-x = public version of brand 'x', where 'x' is a client or some future brand
1.5.0-dev.1 = dev test for 'phet' brand
1.5.0-dev.1-phetio = dev test for 'phetio' brand
1.5.0-dev.1-x = dev test for brand 'x'
1.5.0-rc.1 = RC test for 'phet' brand
1.5.0-rc.1-phetio = RC test for 'phetio' brand
1.5.0-rc.1-x = RC test for brand 'x'

Another advantage of this scheme will become apparent if/when we publish all supported brands for a sim simultaneously. We would want all related files to live in the same directory, with version ids that are clearly similar. For example, for a sim with supported brands 'phet', 'phetio' and 'x':

public versions, all located in 1.0.0/ directory:
1.0.0
1.0.0-phetio
1.0.0-x

dev versions, all located in 1.0.0-dev.15/ directory:
1.0.0-dev.15
1.0.0-dev.15-phetio
1.0.0-dev.15-x

RC version, all located in 1.0.0-rc.1/ directory:
1.0.0-rc.1
1.0.0-rc.1-phetio
1.0.0-rc.1-x

Labeling for discussion at developer meeting.

EDIT (@samreid): fixed a typo

@samreid
Copy link
Member

samreid commented Apr 12, 2017

Prior discussion about how to name the versions is in https://github.com/phetsims/phet-io/issues/623 and it is codified in chipper/js/getVersionForBrand.js

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Apr 12, 2017

The current scheme of inserting "phetio" in front of "dev" and "rc" in the version identifier breaks the detection scheme that we approved in phetsims/joist#406 (comment). One of the hazards of not having a well-defined syntax for version id.

@pixelzoom
Copy link
Contributor Author

For a summary of other problems with the current version id scheme, see phetsims/joist#411 (clean up version identifier).

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Apr 13, 2017

Here's another possible general form that is closer to what we have now. The brand chunk comes before the test chunk:

number[-brand][-test]

E.g.:

1.0.0
1.0.0-phetio
1.0.0-rc.1
1.0.0-phetio-rc.1
1.0.0-dev.1
1.0.0-phetio-dev.1

This would probably require fewer changes to the build process. But imo it's preferable to put the brand chunk last, since it's absence indicates an implicit value ('phet').

@samreid
Copy link
Member

samreid commented Apr 13, 2017

It appears that the semantic versioning rules (which we looked at previously) does support hyphens in pre-release version identifiers. http://semver.org/#spec-item-9 . However, I'm not sure that we should be using "pre-release" style to identify production versions like "1.0.0-phetio".

@samreid
Copy link
Member

samreid commented Apr 13, 2017

Brainstorming: what if instead of changing the version name for phet-io sims, we change the project name.
Currently:
faradays-law 1.0.0-phetio

Proposed:
faradays-law-phetio 1.0.0

This idea was prompted by on the semantic versioning link above.

@samreid
Copy link
Member

samreid commented Apr 13, 2017

image

They are different applications, so doesn't it make sense that they would have different names?

@pixelzoom
Copy link
Contributor Author

They are not different applications. They are the same application with different branding and different features enabled.

@samreid
Copy link
Member

samreid commented Apr 13, 2017

They are not different applications. They are the same application with different branding and different features enabled.

They also have different code (phet-io sim versions include phetio.js internally and wrappers externally).

UPDATE:
build.json shows all of the code differences in the sim:

"phet-io": {
    "preload": [
      "../sherpa/lib/jsondiffpatch-0.1.31.js",
      "../phet-io/js/phetio-query-parameters.js"
    ],
    "phetLibs": [
      "phet-io",
      "phet-io-website"
    ]
  }

@pixelzoom
Copy link
Contributor Author

So... phetio is phet with additional features (and code for those features) added, right?

@samreid
Copy link
Member

samreid commented Apr 13, 2017

Correct, there is added code and different brand metadata.

@samreid
Copy link
Member

samreid commented Apr 13, 2017

Would it be simpler from an engineering standpoint if we only had one file (which supports both phet and phet-io)? If so, we would need @kathy-phet to approve this before moving forward.

The PhET-iO version contains all of the dependencies.json for everything.

This is parallel to the conversation we had earlier about bundling all of the translations into a single file. Maybe bundling all of the "brands" into a single file would provide the same benefits.

CURRENT STATUS: Let's sleep on it and discuss at an upcoming developer meeting.

UPDATE: The PhET and PhET-iO simulations have very different licensing, so likely cannot be combined into a single file.

@samreid
Copy link
Member

samreid commented Apr 20, 2017

We are discussing making phet-io development part of the normal process and unifying the version numbers (and branch names) and having the build process build both artifacts.

@samreid
Copy link
Member

samreid commented Apr 20, 2017

Hence we would take the brand out of the version identifier and branch names.

@jonathanolson and @zepumph and @samreid will work together to investigate how to make this happen.

@samreid
Copy link
Member

samreid commented Apr 20, 2017

As part of #567 we will investigate removing brand from version and branch.

@zepumph
Copy link
Member

zepumph commented Dec 6, 2017

Tagging for dev meeting tomorrow.

@samreid
Copy link
Member

samreid commented Dec 6, 2017

Previously, @kathy-phet has expressed a strong preference for keeping the hyphen where PhET-iO appears in URLs, filenames and other user-visible locations.

@zepumph
Copy link
Member

zepumph commented Dec 6, 2017

So by that we should probably have:

build/
    phet-io/
      faradays-law_all{{?}}phet-io.html
      faradays-law_all{{?}}phet-io{{?}}debug.html

@zepumph
Copy link
Member

zepumph commented Dec 6, 2017

I'm not sure if this relates totally, but we should also try to factor out these names to a single place in chipper/perennial so there isn't a proliferation of assuming that the brand name won't change:
image

@jonathanolson
Copy link
Contributor

Also noting:

  • What should versions be in the About dialog or anything that represents the running sim version? (Presumably it would include the brand always, even if it is -phet).
  • Yotta was using the previous assumption that brands wouldn't use capital letters (among other things), so it will need to be upgraded anyways with whatever we decide.

@ariel-phet
Copy link
Contributor

ariel-phet commented Dec 7, 2017

12/7/17 revisions to #560 (comment) based on decision in #560 (comment):

local and dev directory structure (assuming brands=[phet,phet-io,adapted-from-phet])

build/ (local)
faradays-law/1.0.0/ (dev)
    phet/
      faradays-law_en_phet.html
      faradays-law_zh_CN_phet.html (locale-specific file, built only if specified)
      faradays-law_all_phet.html
      faradays-law_all_phet_debug.html
    phet-io/
      faradays-law_all_phet-io.html
      faradays-law_all_phet-io_debug.html
    adapted-from-phet/
      faradays-law_en_adapted-from-phet.html
      faradays-law_zh_CN_adapted-from-phet.html (locale-specific file, built only if specified)
      faradays-law_all_adapted-from-phet.html
      faradays-law_all_adapted-from-phet_debug.html
 
  1.0.0-dev.1/ and 1.0.0-rc.1/ same as 1.0.0/ 
  (by default, locale-specific files are only built on production)

 // one-off is in the directory name
  1.0.1-dev.1-sonification/
    phet/
      faradays-law_en_phet_sonification.html
      faradays-law_all_phet_sonification.html
      faradays-law_all_phet_sonification_debug.html
    phet-io/
      faradays-law_all_phet-io_sonification.html
      faradays-law_all_phet-io_sonification_debug.html
    adapted-from-phet/
      faradays-law_all_adapted-from-phet_sonification.html
      faradays-law_all_adapted-from-phet_sonification_debug.html

PhET production directory structure

faradays-law/
  1.0.0/
      faradays-law_en.html
      faradays-law_zh_CN.html
      faradays-law_all.html
 

PhET-iO production directory structure

faradays-law/
  1.0.0/
      faradays-law_all_phet-io.html
      faradays-law_all_phet-io_debug.html

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Dec 7, 2017

12/7/17 dev meeting notes, changes reflected in #560 (comment):

• Allow dashes in brand name
• Keep "phet-io" and "adapted-from-phet" brand names
• Changed to underscore ('_') separator
• Continue to us underscore separator in locales (to separate language and country, e.g. "zh_CN")
• Version string (About dialog) and anything named 'version' in sim (e.g. globals) will not include the brand.
• TODO: Document this somewhere when it becomes reality. (see #636)

@mattpen
Copy link
Contributor

mattpen commented Dec 11, 2017

Discussed with @jonathanolson. We think that the build-server should fail and return an error if adapted-from-phet is in the list of brands for a production deploy.

@jonathanolson
Copy link
Contributor

Implemented in the build and dev deploy. Will be waiting on the build-server request format and support for rc/production deploys.

See https://www.colorado.edu/physics/phet/dev/html/chains/1.7.0-dev.20 as an example.

@jonathanolson jonathanolson removed their assignment Dec 12, 2017
mattpen added a commit to phetsims/perennial that referenced this issue Dec 13, 2017
@zepumph zepumph removed their assignment Dec 15, 2017
@zepumph
Copy link
Member

zepumph commented Dec 17, 2017

The commits above should make phet-io wrappers work for the updated version id schema.

@mattpen
Copy link
Contributor

mattpen commented Jan 12, 2018

The build-server requests now accomodate the new version and filename scheme.

@pixelzoom @jonathanolson @samreid - is it ok to close this issue? Should we create documentation first?

@samreid
Copy link
Member

samreid commented Jan 12, 2018

I don't know. What kind of documentation are you thinking of?

@jonathanolson
Copy link
Contributor

Documentation (maybe in a perennial doc/*.md) about versioning is probably recommended.

@pixelzoom
Copy link
Contributor Author

Can someone please summarize (in this issue) the format of the version id scheme that was actually implemented?

@jonathanolson
Copy link
Contributor

Can someone please summarize (in this issue) the format of the version id scheme that was actually implemented?

Notes in SimVersion.js:

 * The canonical description of our general versions:
 *
 * Each version string has the form: {{MAJOR}}.{{MINOR}}.{{MAINTENANCE}}[-{{TEST_TYPE}}.{{TEST_NUMBER}}] where:
 *
 * MAJOR: Sequential integer, starts at 1, and is generally incremented when there are significant changes to a simulation.
 * MINOR: Sequential integer, starts at 0, and is generally incremented when there are smaller changes to a simulation.
 *   Resets to 0 whenever the major number is incremented.
 * MAINTENANCE: Sequential integer, starts at 0, and is incremented whenever we build with the same major/minor (but with different SHAs).
 *   Resets to 0 whenever the minor number is incremented.
 * TEST_TYPE (when present): Indicates that this is a non-production build when present. Typically will take the values:
 *   'dev' - A normal dev deployment, which goes to spot (www.colorado.edu/physics/phet/dev/html/)
 *   'rc' -  A release-candidate deployment (off of a release branch). Also goes to spot only.
 *   anything else - A one-off deployment name, which is the same name as the branch it was deployed from.
 * TEST_NUMBER (when present): Indicates the version of the test/one-off type (gets incremented for every deployment).
 *   starts at 0 in package.json, but since it is incremented on every deploy, the first version published will be 1.
 *
 * It used to be (pre-chipper-2.0) that sometimes a shortened form of the (non-'phet') brand would be added to the end
 * (e.g. '1.3.0-dev.1-phetio' or '1.3.0-dev.1-adaptedfromphet'), or as a direct prefix for the TEST_TYPE (e.g.
 * 1.1.0-phetiodev.1 or 1.1.0-phetio). We have since moved to a deployment model where there are
 * subdirectories for each brand, so this is no longer part of the version. Since this was not used for any production sim
 * builds that we need statistics from, it is excluded in SimVersion.js or its description.
 *
 * Examples:
 *
 * 1.5.0 - Production simulation version (no test type). Major = 1, minor = 5, maintenance = 0
 * 1.5.0.rc.1 - Example of a release-candidate build version that would be published before '1.5.0' for testing.
 * 1.5.0.dev.1 - Example of a dev build that would be from master.
 * 1.5.0.sonification.1 - Example of a one-off build (which would be from the branch 'sonification')

@jonathanolson
Copy link
Contributor

Closing, since this versioning has been fully implemented. If anyone needs anything else, please re-open.

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