feat: [UEPR-15] Updated script to fix build issues and refactored logic for merging branches #2

MiroslavDionisiev · 2024-07-08T10:29:27Z

Jira Ticket

Proposed Changes

Changed the logic for creating monorepo branches and merging code. Added logic that fixes wrong paths after the merge in the monorepo and ensures everything builds properly.

…ic for merging branches

sashodada · 2024-07-09T07:21:41Z

scripts/build-monorepo.sh

 ALL_REPOS="
-    scratch-audio \
-    scratch-desktop \
    scratch-gui \
-    scratch-l10n \
-    scratch-paint \
-    scratch-parser \
    scratch-render \
-    scratch-sb1-converter \
-    scratch-semantic-release-config \
-    scratch-storage \
    scratch-svg-renderer \
-    scratch-translate-extension-languages \
    scratch-vm \


Even though they are tracked externally, should we leave the removed repos in a comment somewhere around here for future reference? There is a commented out definition of ALL_REPOS below we could probably reuse. Maybe also changing the comment above the ALL_REPOS definition to fit the current state of the script would be helpful as well

sashodada · 2024-07-09T07:33:29Z

scripts/build-monorepo.sh

+default_branch () {
+    BRANCH=""
+
+    if [ -z "$(git -C "${BUILD_TMP}/${REPO_NAME}" branch --list "$BRANCH")" ]; then


Can this conditional be skipped since the first check is against an empty branch name?

sashodada · 2024-07-09T07:34:57Z

scripts/build-monorepo.sh

+                jq  "del(.dependencies.\"$DEP\")" "${BUILD_OUT}/workspaces/${REPO}/package.json"  | sponge "${BUILD_OUT}/workspaces/${REPO}/package.json"
                DEPS="$DEPS $DEP@*"
            fi
            if jq -e .devDependencies.\"$DEP\" "${BUILD_OUT}/workspaces/${REPO}/package.json" > /dev/null; then
-                REMOVEDEPS="$REMOVEDEPS $DEP"
+                jq  "del(.devDependencies.\"$DEP\")" "${BUILD_OUT}/workspaces/${REPO}/package.json"  | sponge "${BUILD_OUT}/workspaces/${REPO}/package.json"
                DEVDEPS="$DEVDEPS $DEP@*"
            fi
            if jq -e .optionalDependencies.\"$DEP\" "${BUILD_OUT}/workspaces/${REPO}/package.json" > /dev/null; then
-                REMOVEDEPS="$REMOVEDEPS $DEP"
+                jq  "del(.optionalDependencies.\"$DEP\")" "${BUILD_OUT}/workspaces/${REPO}/package.json"  | sponge "${BUILD_OUT}/workspaces/${REPO}/package.json"
                OPTDEPS="$OPTDEPS $DEP@*"
            fi
            if jq -e .peerDependencies.\"$DEP\" "${BUILD_OUT}/workspaces/${REPO}/package.json" > /dev/null; then
-                REMOVEDEPS="$REMOVEDEPS $DEP"
+                jq  "del(.peerDependencies.\"$DEP\")" "${BUILD_OUT}/workspaces/${REPO}/package.json"  | sponge "${BUILD_OUT}/workspaces/${REPO}/package.json"


Are the duplications in these statements worth extracting in a variable?

…anch

cwillisf

Looking good! I have some questions about potential simplification, but if there's a reason to keep it the way it is then I think that's fine :)

cwillisf · 2024-07-16T17:48:25Z

scripts/build-monorepo.sh

+
+    GH_PAGES_BRANCH="gh-pages"
+
+    clone_repository $REPO_NAME


Is there a way to skip this clone step, since we will already have cloned the repo in add_repo_to_monorepo? Or is there a reason it needs to be cloned again?

I'm wondering if add_gh_pages could instead be a special branch handler in the DEST_BRANCH loop in add_repo_to_monorepo. Maybe something like:

gh-pages) add_gh_pages $REPO_NAME continue ;;

...or possibly even just an "if the branch is gh-pages then create it a little bit differently" clause in the create the destination branch if it doesn't exist section of add_repo_to_monorepo?

The reason is that after we changed once the root to be treated as directory with --to-subdirectory-filter the rerun of the command with different values keeps the structure created by the first call. This may have a better solution but I decided to not try and over-engineer it.

cwillisf · 2024-07-16T17:54:36Z

scripts/build-monorepo.sh

+    if [ -z "$(git -C "$BUILD_OUT" branch --list "$GH_PAGES_BRANCH")" ]; then
+        # create the destination branch if it doesn't exist
+        git -C "$BUILD_OUT" symbolic-ref HEAD "refs/heads/${GH_PAGES_BRANCH}"
+        rm -rf "${BUILD_OUT}/.git/index"
+        git -C "$BUILD_OUT" clean -fdx
+        git -C "$BUILD_OUT" checkout develop .gitignore
+        git -C "$BUILD_OUT" add .
+        git -C "$BUILD_OUT" commit --allow-empty -m "Initial commit for github pages"
+    else


Is this different only because a new gh-pages branch shouldn't contain the source code (and other contents) from develop? Or is there more going on here?

My intention was to create an orphan branch so that its history is not associated with the branch it is checked out of. However I didn't like the state of the branch after git checkout --orphan and decided to go with this solution.

cwillisf · 2024-08-08T14:10:42Z

scripts/build-monorepo.sh

+    nvm install < .nvmrc
+    nvm use < .nvmrc


I don't think nvm uses stdin at all, so I believe these redirects are unnecessary. As long as .nvmrc is present in the current working directory, this should work the same:

Suggested change

nvm install < .nvmrc

nvm use < .nvmrc

nvm install

nvm use

cwillisf · 2024-08-08T14:12:54Z

scripts/git-batch-push.sh

+        git -C "$BUILD_OUT" push $REMOTE ${h}:refs/heads/$BRANCH --force
+    done
+    # push the final partial batch
+    git -C "$BUILD_OUT" push $REMOTE HEAD:refs/heads/$BRANCH --force


Is --force necessary? It makes me nervous about the possibility of something going wrong and causing this to force-push to the wrong branch or repo 😅

If possible, I'd prefer to remove --force here and instead require a human to prepare the remote branch, maybe with a manual force-push to a base commit.

Yeah, my idea was this value to be replaced with where the fork of the one running the script is, however I didn't make it obvious.

[UEPR-19] Added workflow for building and testing packages

…extensions toolbox categories (#2)

feat: [UEPR-15] Updated script to fix build issues and refactored log…

118f558

…ic for merging branches

sashodada approved these changes Jul 9, 2024

View reviewed changes

MiroslavDionisiev added 2 commits July 11, 2024 14:16

feat: [UEPR-15] Refactored code and added creation of github pages br…

3fe55dc

…anch

feat: [UEPR-15] Removed unnecessary destination branch

57862c1

cwillisf reviewed Jul 16, 2024

View reviewed changes

MiroslavDionisiev added 4 commits July 18, 2024 17:58

refactor: [UEPR-15] Change variable interpolation

1549acb

refactor: [UEPR-15] Removed redundant switch check

4c68e63

refactor: remove main branch from list and add commit message

85fb563

feat: [UEPR-15] Added script to push in batches

6037233

MiroslavDionisiev requested a review from cwillisf July 23, 2024 09:59

MiroslavDionisiev added 2 commits August 7, 2024 12:58

refactor: [UEPR-15] creating main branch from develop

5a7e74c

refactor: [UEPR-15] added logic to attach organization name to packages

2fcc50c

cwillisf requested changes Aug 8, 2024

View reviewed changes

refactor: [UEPR-15] removed use of stdin in nvm and removed force push

b7314ee

MiroslavDionisiev requested a review from cwillisf August 9, 2024 08:36

cwillisf approved these changes Aug 9, 2024

View reviewed changes

chore: [UEPR-15] resolved merge conflicts

91af2b2

MiroslavDionisiev merged commit ab70c28 into scratchfoundation:develop Aug 12, 2024

MiroslavDionisiev added a commit that referenced this pull request Sep 11, 2024

Merge pull request #2 from MiroslavDionisiev/UEPR-19

897a7d3

[UEPR-19] Added workflow for building and testing packages

cwillisf pushed a commit that referenced this pull request Feb 20, 2025

fix: use toolboxitemid instead of id as the identifier attribute for …

28ea54a

…extensions toolbox categories (#2)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: [UEPR-15] Updated script to fix build issues and refactored logic for merging branches #2

feat: [UEPR-15] Updated script to fix build issues and refactored logic for merging branches #2

Uh oh!

MiroslavDionisiev commented Jul 8, 2024 •

edited by atlassian bot

Loading

Uh oh!

sashodada Jul 9, 2024

Uh oh!

sashodada Jul 9, 2024

Uh oh!

sashodada Jul 9, 2024

Uh oh!

cwillisf left a comment

Uh oh!

cwillisf Jul 16, 2024

Uh oh!

cwillisf Jul 16, 2024

Uh oh!

MiroslavDionisiev Jul 17, 2024

Uh oh!

cwillisf Jul 16, 2024

Uh oh!

MiroslavDionisiev Jul 17, 2024

Uh oh!

cwillisf Aug 8, 2024

Uh oh!

cwillisf Aug 8, 2024

Uh oh!

MiroslavDionisiev Aug 9, 2024

Uh oh!

Uh oh!

feat: [UEPR-15] Updated script to fix build issues and refactored logic for merging branches #2

feat: [UEPR-15] Updated script to fix build issues and refactored logic for merging branches #2

Uh oh!

Conversation

MiroslavDionisiev commented Jul 8, 2024 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira Ticket

Proposed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwillisf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MiroslavDionisiev commented Jul 8, 2024 •

edited by atlassian bot

Loading