Skip to content

Makefile refactor #408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

akosthekiss
Copy link
Member

A loosely coupled, intentionally non-squashed set of patches improving the Makefile. Key property of the patches is that they don't change any current default behaviour (but extend possibilities, improve readability of recipes, or fix latent bugs).

@egavrin egavrin added the enhancement An improvement label Jul 15, 2015
@egavrin
Copy link
Contributor

egavrin commented Jul 15, 2015

Related issue: #108

@egavrin
Copy link
Contributor

egavrin commented Jul 15, 2015

Good to me

@akosthekiss
Copy link
Member Author

Ah, but I got it wrong. tee is a tricky beast and swallows the exit code of the command earlier in the pipe. (Which would cause always-successful builds from make's perspective.) Will update the PR with fix ASAP.

@akosthekiss akosthekiss force-pushed the make-refactor branch 2 times, most recently from fea447a to 4c72a8e Compare July 16, 2015 11:19
@akosthekiss
Copy link
Member Author

FYI: updated and rebased

@egavrin
Copy link
Contributor

egavrin commented Jul 16, 2015

@akiss77 is possible to split logs into two streams stdout and stderr and put them into separate logs? The idea is to print all errors on screen after in case of build failure - not to analyze logs by eye.

@egavrin egavrin self-assigned this Jul 16, 2015
@akosthekiss
Copy link
Member Author

@egavrin can/will experiment. Could we have that as a follow-up, however? This patch set keeps everything working as was before by default. The log splitting approach would cause a change in the default behaviour.

@egavrin egavrin assigned zherczeg and unassigned egavrin Jul 16, 2015
@egavrin
Copy link
Contributor

egavrin commented Jul 16, 2015

@akiss77 of course :)

Good to me.

@ruben-ayrapetyan
Copy link
Contributor

Looks good to me

@akosthekiss
Copy link
Member Author

Seen two LGTMs, thanks, I'll push.

@akosthekiss
Copy link
Member Author

Merged at 6038173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants