-
Notifications
You must be signed in to change notification settings - Fork 5
Safer clean #115
base: master
Are you sure you want to change the base?
Safer clean #115
Conversation
export I_AM_CLUMSY=1
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.
Looks good other than comments 👍
# HACK: Can't use -e option here because it's not supported by our Jenkins | ||
@git clean -fxd | ||
# clean just untracked files if not on CircleCI | ||
@if [ -z $(CIRCLECI) ]; then git clean -Xfd; else git clean -xfd; fi |
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.
Isn't this logic inverted? I.e. We only want to clean .gitignored
files locally and all in CI?
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.
@phamann the -z
means it "is null, that is, has zero length"
so, when CIRCLECI
is not set then we use -Xfd
and when CIRCLECI
is set then we use -xfd
That what we want, right?
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.
Yep :) so my only comment now is that the conditional is confusing (at least for me). Can it instead read:
@if [ -e $(CIRCLECI) ]; then git clean -xfd; else git clean -Xfd; fi
@@ -35,8 +35,12 @@ CONFIG_VARS = curl -fsL https://ft-next-config-vars.herokuapp.com/$1/$(call APP_ | |||
|
|||
# clean | |||
clea%: | |||
# HACK: Can't use -e option here because it's not supported by our Jenkins | |||
@git clean -fxd | |||
# clean just untracked files if not on CircleCI |
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.
"clean just gitignored files if not on CircleCI"
@if [ -z $(CIRCLECI) ]; then git clean -Xfd; else git clean -xfd; fi | ||
@$(DONE) | ||
|
||
cleanx: |
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.
This will have to be named something different because on the clean%
, make cleanx
would just call make clean
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.
Oh actually it turns out it doesn't (a fully specified name does take precedence) buuut I would still call it something else like nuke
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.
@leggsimon the peace hippy that I am, I feel it my duty to encourage people not to nuke...
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.
You're the one that wants it 😉
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.
But seriously I think it should be more clearly different
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.
@leggsimon any suggestion for an alternative word to nuke? Preferably one that isn't violent... and 3 or 4 letters
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.
make new
make tidy
make fresh
make i-give-up-take-me-back-to-how-it-was-before
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.
Great ideas, @leggsimon I really like the make fresh
one. So, I've just done that.
I think we're good to go now?
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
FYI should update the README as well |
@$(DONE) | ||
|
||
# start completely afresh... clean all non-indexed files including those gitignored | ||
fresh: |
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.
fres%
?
Is this still happening? |
use
make cleanx
for the full clean