script: double change #89
Conversation
save_the_dev function that stashes current work to reapply if after sync do get bleeding edge patches on everything saving the dev's ass; adding some humor to the interation with user;
|
#86 it's there too. |
separating stash and stash applying via two functions now
build-dakkar.sh
Outdated
| @@ -100,14 +101,14 @@ function get_rom_type() { | |||
| mainrepo="https://github.com/PixelExperience/manifest" | |||
| mainbranch="oreo-mr1" | |||
| localManifestBranch="android-8.1" | |||
| treble_generate="pixel" | |||
| treble_generate="" | |||
There was a problem hiding this comment.
Not that it really matters, but I guess they don't need extra_make_options either
build-dakkar.sh
Outdated
| } | ||
|
|
||
| function restore_the_dev() { | ||
| if [ -d device/phh/trble ] && [ -z "$stashstatus" ];then |
| @@ -300,6 +301,25 @@ function jack_env() { | |||
| fi | |||
| } | |||
|
|
|||
| function save_the_dev() { | |||
| if [ -d device/phh/treble ];then | |||
There was a problem hiding this comment.
Please invert this to reduce indentation and for better clarity
There was a problem hiding this comment.
invert?
What do you mean ?
There was a problem hiding this comment.
something like
[ -d device/phh/treble] && return
build-dakkar.sh
Outdated
|
|
||
| function restore_the_dev() { | ||
| if [ -d device/phh/trble ] && [ -z "$stashstatus" ];then | ||
| cd device/phh/treble |
There was a problem hiding this comment.
Please use a subshell, instead of cd xxx; .... ; cd $curpath which is error prone
There was a problem hiding this comment.
as said earlier, should be already in subshell since this is a function
build-dakkar.sh
Outdated
| function save_the_dev() { | ||
| if [ -d device/phh/treble ];then | ||
| cd device/phh/treble | ||
| if git stash -u | grep -q 'No local changes to save';then |
| @@ -300,6 +301,25 @@ function jack_env() { | |||
| fi | |||
| } | |||
|
|
|||
| function save_the_dev() { | |||
| if [ -d device/phh/treble ];then | |||
| cd device/phh/treble | |||
There was a problem hiding this comment.
Please use a subshell, instead of cd xxx; .... ; cd $curpath which is error prone
There was a problem hiding this comment.
a function doesn't automatically assume to execute stuff in subshell?
There was a problem hiding this comment.
if that's the case, you can just drop the cd $oldpwd
There was a problem hiding this comment.
stuff is, the execution of this script is in the root of sync, if I drop the cd it will stash on the whole rom files and not only on DT
build-dakkar.sh
Outdated
| if [ -d device/phh/treble ];then | ||
| cd device/phh/treble | ||
| if git stash -u | grep -q 'No local changes to save';then | ||
| stashstatus=blank |
There was a problem hiding this comment.
"blank" when you're later checking for "empty" is weird -_-'
build-dakkar.sh
Outdated
| function save_the_dev() { | ||
| if [ -d device/phh/treble ];then | ||
| cd device/phh/treble | ||
| if git stash -u | grep -q 'No local changes to save';then |
There was a problem hiding this comment.
I think it would make more sense to if ! git stash -u;then instead
build-dakkar.sh
Outdated
| function restore_the_dev() { | ||
| if [ -d device/phh/trble ] && [ -z "$stashstatus" ];then | ||
| cd device/phh/treble | ||
| git stash apply |
There was a problem hiding this comment.
Considering this might/will fail, some error handling should be done here
There was a problem hiding this comment.
If this fails due to set -e should automatically stop the whole script notifying the user about the failed stash, so I dunno what we should handle further.
dropping $curpath as function should already run in subshells;
typo, trble => treble; function init_main_repo_ref: ask user if it has a reference path to sync from and let him do that in the case.
save_the_dev function that stashes current work to reapply if after sync do get bleeding edge patches on everything saving the dev's ass;
adding some humor to the interation with user;
fix some roms makefiles assign;