Conversation
|
This looks cool! So I have 2 thoughts:
Basically have a directory where you can drop your user specific includes that don't get committed to git. Maybe I'm 96% that should work - but what do you think of the idea? It should allow for nice flexibility without messing up things across the board for everyone. |
|
ok but we keep the $(DOCKER_BUILD_ARGS) for each docker build stage ? I could also use an env var ? |
|
Yeah, let's definitely do that, for sure! Maybe the |
|
Also, I feel like there should be a better name than |
|
tested your solution and it works very well. Should I rename the folder ? |
.gitignore
Outdated
| *.o | ||
| tmp No newline at end of file | ||
| tmp | ||
| includes.d/ No newline at end of file |
There was a problem hiding this comment.
So I'm thinking this should be
includes.d/*
Then we can drop a nice README.md in there saying "Directory for your personal includes that won't get added to the git repository" (or find better language)
There was a problem hiding this comment.
heh, would also need a !/includes.d/README.md
Otherwise the README would get ignored 😊
There was a problem hiding this comment.
I'm looking at this it's not working
There was a problem hiding this comment.
Works for .gitignore - is the path correct?
(Maybe drop a .gitignore into the local-includes dir, and do it from there, as easier?)
There was a problem hiding this comment.
build/local-includes/*
!build/local-includes/README.md
that works
There was a problem hiding this comment.
good with this or should I drop a new ignore file ?
build/Makefile
Outdated
| include ./includes/$(osinclude) | ||
|
|
||
| # personal includes, excluded from the git repository | ||
| -include ./includes.d/*.mk |
There was a problem hiding this comment.
I don't know if I'm sold on "includes.d" as a directory name.
user-includes ? I'm not sure. Any ideas?
There was a problem hiding this comment.
well .d was at first to deferentiate directory from other files when names were the same.
https://unix.stackexchange.com/questions/4029/what-does-the-d-stand-for-in-directory-names
There was a problem hiding this comment.
I'm not helping you :)
There was a problem hiding this comment.
Hah! Yeah, I looked at the same thing - which made me think the .d wasn't a good name.
includes-local , local-includes ?
I'm leaning towards local as a name, WDYT?
There was a problem hiding this comment.
local-includes looks nice
There was a problem hiding this comment.
SGTM. local-includes it is.
|
wait a sec forget to rename SHELL_ARGS |
|
but it's not consistent you used ARGS for docker also: Why do you need that one ? EDIT: for minikube-install found it |
|
I choosed to rename to DOCKER_RUN_ARGS instead because that's what it is and we could propagate if required to other target using docker run. (for example I will need proxy for gcloud-init) |
|
This looks good, just please rebase down to a single commit, and I'll approve. |
markmandel
added
the
area/build-tools
|
@Kuqd Did you want to rebase this down to a single commit? I can always try out the Github "squash and merge" button - I've not tried it before 🤷♂️ |
cyriltovena
force-pushed
the
cleanup/remove-entrypoint
branch
from
bc12396 to
5a38c01
Compare
|
yes sorry I was busy, I just did it, I'm using this button a lot on our internal gitlab ! Though the merge just created another commit, so I have to squash again. EDIT: you should be good to go ! |
cyriltovena
force-pushed
the
cleanup/remove-entrypoint
branch
from
7123daf to
a2f2730
Compare
|
@markmandel I don't have access to your CI to see what's wrong ! just by rebasing ? |
|
@Kuqd looks like cloud builder fell over for some reason. Fired off a new build. At some point in the future, I need to implement pushing of results to PRs. (One thing at a time) |
2 participants
Removed all entrypoints in the makefile.
I've tested all changed targets except the gcloud cluster creation as I don't have a gcloud account (poor me).
I've left my proxy args for build, but I can remove it if you want though it will be transparent for you.
Resolves #28