-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Don't fail an incremental build if can't find a merge base #444
Conversation
Added an additional check and warning message
script/incremental_build.sh
Outdated
FLUTTER_CHANGED_PACKAGES=`git diff --name-only $BRANCH_BASE_SHA HEAD | grep -o "packages/[^/]*" | sed -e "s/packages\///g" | sort | uniq | paste -s -d, -` | ||
|
||
FLUTTER_CHANGED_GLOBAL=0 | ||
FLUTTER_CHANGED_GLOBAL="" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional to first assign 0 and then empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! It's a typo. It suppose to be FLUTTER_CHANGED_PACKAGES
. Updated the PR.
script/incremental_build.sh
Outdated
FLUTTER_CHANGED_GLOBAL=`git diff --name-only $BRANCH_BASE_SHA HEAD | grep -v packages | wc -l` | ||
FLUTTER_CHANGED_PACKAGES=`git diff --name-only $BRANCH_BASE_SHA HEAD | grep -o "packages/[^/]*" | sed -e "s/packages\///g" | sort | uniq | paste -s -d, -` | ||
else | ||
echo "Cannot find a merge base for the current branch to run an incremental build!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add to this message what would be required to make the build incremental (e.g. rebase, maybe?)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one comment.
@goderbauer added an actionable message 🙏 |
LGTM Travis failed, I assume its a flake. I restarted it, will merge on green. |
Added an additional check and a warning message.
This change addresses an issue experienced in #423.
/cc @goderbauer