Polish fake-aws stuff #305
Conversation
neongreen
force-pushed
the
polish
branch
2 times, most recently
from
18fd28b
to
1407f5a
Compare
|
Okay, this is probably the final design for libzauth-related things. Basically, if /usr/local is not writable then I always install libzauth locally, and always look for it locally as well. |
|
Also, when libzauth is not found during |
|
|
Makefile
Outdated
| @@ -10,11 +10,11 @@ init: | |||
|
|
|||
| .PHONY: install | |||
| install: init | |||
| stack install --pedantic --test --local-bin-path=dist | |||
| stack install --pedantic --test --no-run-tests --local-bin-path=dist | |||
There was a problem hiding this comment.
What's your rationale for not running tests upon install ? I'm fine with not running them upon fast, but if they don't run upon install, they never run. CI would need to updated to add a line (which would be fine as such, but I'd like to know the reasoning behind not wanting to run tests upon install first).
There was a problem hiding this comment.
The docs told me to do make services so that's what I did. Then it was running tests all the time, it was annoying, and I asked Tiago whether I could remove it. There wasn't much reasoning involved 🙂
I don't have a strong opinion about running vs. not running them on install, but it seems counter-intuitive to me that install should test anything, so I'm +0.5 on keeping the change and updating the CI instead. If you're okay with it, I'll do it.
There was a problem hiding this comment.
In my opinion, there should be one way to compile faster (no optimizations, no tests), and one way to compile slower (with more validations, i.e. tests; and resulting in optimized binaries). That's currently provided via make==make fast and make install respectively. make services (currently) depends on make install.
As I said, I think removing the tests on make install is dangerous, as it means they will probably never run (people will forget to run them - nothing indicates they can, or they should - the docs don't mention anything here). I'm not sure why you needed to run make services repeatedly, you were probably only trying to compile nginz (which a cd nginz && make would do, too, and be faster.)
There was a problem hiding this comment.
As I said, I think removing the tests on make install is dangerous, as it means they will probably never run (people will forget to run them [...])
But if CI runs tests (as it should), why do people have to run them too?
services/nginz/Makefile
Outdated
| .PHONY: check-deps | ||
| check-deps: | ||
| PKG_CONFIG_PATH=$(EXTRA_PKG_PATH) pkg-config --exists libzauth || { echo -e "\n\033[0;31m The 'libzauth' library was not found\033[0m\n pkg-config path = $(EXTRA_PKG_PATH)\n\n Suggestion: run 'make libzauth' on the top level\n"; exit 1; } | ||
|
|
There was a problem hiding this comment.
How about adding another target here:
.PHONY: libzauth
libzauth:
$(MAKE) -C ../../libs/libzauth installAnd, instead of printing a warning in the || { .. block, doing a || { echo -e "The 'libzauth' library was not found, attempting to install it..." && make libzauth ?
|
@jschaul please re-review |
No description provided.