search

phetsims / perennial

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

MR process should get every release branch and cache it #318

Open zepumph opened 1 year ago

zepumph commented 1 year ago

From https://github.com/phetsims/perennial/issues/317, ReleaseBranch.getMaintenanceBranches() takes a long time to load, so it would be awesome to have that list cached for an MR. I think putting it into the .maintenance.json makes the most sense, and then adding doc to potentially report if there is a new ReleaseBranch if the MR goes too long and you need to recalculate.

zepumph commented 1 year ago

Ok. So I added serilization for allReleaseBranches into the Maintenance prototype. Now we moved getMaintenanceBranches() from ReleaseBranch into Maintenance plus caching if the list is empty. If you ever wanted to "break" the cache you can supply an option to reload things. I believe this will save me ~1 hour every maintenance release. I'm very excited, but I don't feel super confident about all the ramifications of these changes. Can we have a cursory review be at a high priority just in case another MR comes up soon.

zepumph commented 1 year ago

Oops, @AgustinVallejo and I just ran into the above bug fix because I forgot to change all the usages that were filtering from the repo string instead of using the whole ReleaseBranch. Back to you @jonathanolson.

zepumph commented 1 year ago

@samreid and I ran into a caching problem here where the save of the cache is overwritten by another instance of maintenance loaded before getMaintenanceBranches was called, and then overwritten onto the save file once we have gotten the release branches.

Also there are some TODOs pointing to this issue, so I'll take a look at a few of these.

zepumph commented 1 year ago

Ok. I also removed the issue's TODOs. I feel like we could close this, but for completeness I would rather assign to @jonathanolson to sign off on caching things. 2 bug fixes have now come from using this change.