phetsims / perennial

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

main-pull-status.js might be running git operations on repos in parallel #321

Open jessegreenberg opened 1 year ago

jessegreenberg commented 1 year ago

While doing an automated release process https://github.com/phetsims/tasks/issues/1123, I tried to use master-pull--status.js --allBranches to pull all release branches from remote. I ran into an error where I had uncommitted changes in brand graphing-quadratics-1.0 and scenery molecules-and-light-1.5.

I wonder if this Promise.all() is the problem: await Promise.all( repos.map( repo => getStatus( repo ) ) );

I am going to try to run it again with so it waits for each repo individually.

  for ( const repo of repos ) {
    await getStatus( repo );
  }
jessegreenberg commented 1 year ago

Yes that worked. Here is the patch:

Subject: [PATCH] Remove opt out of Interactive Highlights, see https://github.com/phetsims/friction/issues/201
---
Index: js/scripts/master-pull-status.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/scripts/master-pull-status.js b/js/scripts/master-pull-status.js
--- a/js/scripts/master-pull-status.js  (revision 8f5beec14684b1d85e7f4ac05bd5d1c29bdc380e)
+++ b/js/scripts/master-pull-status.js  (date 1684433025053)
@@ -104,7 +104,9 @@
 };

 ( async () => {
-  await Promise.all( repos.map( repo => getStatus( repo ) ) );
+  for ( const repo of repos ) {
+    await getStatus( repo );
+  }

   repos.forEach( repo => {
     process.stdout.write( data[ repo ] );

@jonathanolson is this OK? It might be a little slower but it avoided the problem. I didn't commit because I am not sure where all this script is used.

jonathanolson commented 1 year ago

That's weird, can you try running the original again? Is this reproducible? I call this script multiple times a day (without the --allBranches, maybe that's triggering a problem)

jonathanolson commented 1 year ago

Also, it's meant to be parallel, so that it minimizes the time needed in order for people to pull repos.

jessegreenberg commented 1 year ago

Yes, this is reproducible on Windows 10. I just ran it on two difference machines and both finished with unstaged changes in various repos.

I tried without --allBranches and did not see the problem.