This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Keep patch file permissions in mklog


> thanks for the review.

Np, I'm personally happy to see that script is useful.

> I've now interpreted it such that --inline prints to stdout what it
> would print to the patch file otherwise, that is, both log and patch.
> Printing just the log to stdout can be already be achieved by not using
> --inline.

Could you add a note in the help message?

+if ($#ARGV == 1 && ("$ARGV[0]" eq "-i" || "$ARGV[0]" eq "--inline")) {

I'd do >= 1 but that's a question of personal preference.

+if ($inline && $diff ne "-") {
+	$tmp = `mktemp`;
+	if ($? != 0) {
+		die "Could not generate temp file";
+	}

IMHO better use consistent style: system() or ticks with $?.
Or even better, encapsulate environment calls in a subfunction which would check return code and return stdout.

BTW you may want to wait for Diego's and Trevor's comments (Diego is the maintainer and approver for this code).

-Y


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]