phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
2 stars 5 forks source link

Code Review: build-server parallel checkouts changes #332

Open mattpen opened 1 year ago

mattpen commented 1 year ago

@chrisklus is going to describe what needs to be reviewed and assign this to @jonathanolson when ready.

chrisklus commented 1 year ago

This issue is for reviewing work done in https://github.com/phetsims/chipper/issues/1112 and https://github.com/phetsims/perennial/issues/330.

Please review the following commits in the order below:

https://github.com/phetsims/perennial/commit/3e1ad117fc6812b37a3261d0619f125f289467b7 https://github.com/phetsims/perennial/commit/6fb1ab83086db752f2e4029c600a684ffd23a819 https://github.com/phetsims/perennial/commit/1309fd20bdb1503058479e40461d856f4b10c5c6 https://github.com/phetsims/perennial/commit/ed5f7250112129fd2ab328109fde2184a60feb19 https://github.com/phetsims/perennial/commit/a911b3f4a61e112837350aa5c23f3e45d57e6bde https://github.com/phetsims/perennial/commit/aca3e0e43424e40ab8cf74edcf915d0b6ad47b50 https://github.com/phetsims/perennial/commit/4f25e7a7c222ddf4f56893675743ea355f4d0d42 https://github.com/phetsims/perennial/commit/a3510b9d2c68042d81f724bd83a585d8ef514892 https://github.com/phetsims/perennial/commit/5cfb8d17503ac5aae1cf926dcbb47f1ecf28758d https://github.com/phetsims/perennial/commit/0a9f16e549d7100d007b5339482266069692762f https://github.com/phetsims/perennial/commit/ed136dd2192e166bca38347335cf2db10b95a110 https://github.com/phetsims/perennial/commit/a7796f348c0ad0a67326facf316234bf67f7ff09

The goal of these changes was to remove relative links to where the master checkouts formerly lived and to likewise remove any incorrect dependencies on the master checkouts during builds.

To have confidence that things are working correctly, we restructured the sub-directories of /data/share/phet/build-server on phet-server2 to contain image-repos (where the master checkouts now live, used only to get sim images), perennial, and release-branches. This means that anything referencing a master checkout in the original location will fail.

Successful tests of these changes are outlined in https://github.com/phetsims/perennial/issues/330#issuecomment-1601778199.

It looks like https://github.com/phetsims/chipper/issues/1112 still needs to be wrapped up - either with a review of https://github.com/phetsims/perennial/commit/a3257f46b7ad1e77742ac03742b7ae3658cf741a, or a better solution as mentioned in https://github.com/phetsims/chipper/issues/1112#issuecomment-1562034873.

Please let me know if you'd like to do any of this review together or need more explanation for any parts.