phetsims/build-an-atom

Why is there a 1.7 branch? And what should be done with it?

Closed this issue · 9 comments

jbphet commented

I'm currently working on a fix for an issue which will require a sim-specific maintenance release, see #180. I just checked the currently live version, and it is from the 1.6 branch, a 1.7 branch also exists, and I'm not clear as to why. It appears to have been created by @zepumph about two years ago in March of 2021 if I'm interpreting this information correctly. There was an RC.2 published from this branch, but I don't see anything after that.

I will go ahead an propagate fixes to both the 1.6 and 1.7 branches, but if we don't need this branch we should remove it to reduce the maintenance version. If we do need it, and we aren't going to publish it any time soon, we should log an issue that explains what's going on with it.

jbphet commented

Assigning to @zepumph, since it seems like he created it. If not, please assign back to me and I'll do some more detective work on it.

jbphet commented

It turned out to be a pretty involved task to propagate the changes for #180 to the 1.6 branch because the branch is old enough that I couldn't cherry pick the changes. I'd really rather not have to fix up the 1.7 branch too, so I'm going to label this as high priority so that I can get the information necessary to decide whether I can close #180.

zepumph commented

Paper trail is https://github.com/phetsims/special-ops/issues/189, in the future we should make an issue called branch: 1.7 or likely just make a one-off for these needs. We will keep the 1.7 branch to support the data study. It isn't worth deleting.

Please do not MR this though. Also it would be nice if BAA 1.7 could never be MR'd again. @jonathanolson can you opt out of this generally? I see you have made many many MRs on the 1.7 branch in the last few years.

jbphet commented

Thanks for the update @zepumph - I will not update this branch with the fixes for #180 and will not perform a maintenance release (MR) of this branch for that issue.

jonathanolson commented

It's possible to add phet.ignoreForAutomatedMaintenanceReleases as true in the package.json on the branch, so it will be ignored by the maintenance tools.

zepumph commented

Oh wonderful! Thanks so much for the flag.

zepumph commented

@chrisklus and I just went to do a MR for number-play, and checkBranchStatus showed an update for BAA 1.7 still:


build-an-atom 1.7
  [INFO] Potential changes (dependency is not previous commit)
  [INFO] f5cfa96235b4a3b684735e0c6ca4ce2b5137cb78 862f7233b3c40dfa7e102785742eeb71b0fa026c c23f376b9fd92f6d620320db1a928759a7b40947

I'm confused because I see the ignore flag on the 1.7 branch.

https://github.com/phetsims/build-an-atom/blob/1.7/package.json#L26

@jonathanolson can you help me understand this? Is this expected to still get printed?

jonathanolson commented

Should be fixed in the above commit. We were only checking master for that flag apparently (oops).

zepumph commented

Ahh, excellent. Thanks! I just ran checkBranchStatus() again and confirmed the fix. Closing

image