freebsd-update: open $PAGER only if necessary
ClosedPublic
Actions

Authored by aryeeteygerald_rogers.com on Jan 21 2019, 5:55 PM.

Details

Reviewers

emaste
delphij

Summary

Previously, the $PAGER was opened even with no contents.
Applies patch to no longer do this.

PR: 194547,208497

Test Plan

Verified expected behaviour in interactive sh shell

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 22067
Build 21295: arc lint + arc unit

Event Timeline

aryeeteygerald_rogers.com created this revision.Jan 21 2019, 5:55 PM

Harbormaster completed remote builds in B22067: Diff 53058.Jan 21 2019, 5:55 PM

emaste added subscribers: delphij, jilles.Jan 21 2019, 9:21 PM

I like this change in principle, but I think you could simplify the code a little bit (see my comments in line).

usr.sbin/freebsd-update/freebsd-update.sh
1953	Could you please refactor this to something like: cat - modifiedfiles <<- EOF \| ${PAGER} The following files are affected by updates, but no changes have ... EOF instead?

This revision now requires changes to proceed.Jan 22 2019, 5:26 AM

Use HEREDOC/cat instead of multiple echos

This is indeed much nicer.

Harbormaster completed remote builds in B22073: Diff 53086.Jan 22 2019, 3:43 PM

Remove unnecessary diff junk.

aryeeteygerald_rogers.com marked an inline comment as done.Jan 22 2019, 5:11 PM

One last change request -- could you please use ${PAGER} instead of $PAGER while there? The change looks otherwise fine to me.

This revision is now accepted and ready to land.Jan 24 2019, 5:51 AM

Forgot the review link; committed in rS343407

Revision Contents
Changeset List

Path

Size

usr.sbin/

freebsd-update/

freebsd-update.sh

53 lines

Diff 53058

View Options

usr.sbin/freebsd-update/freebsd-update.sh

freebsd-update: open $PAGER only if necessaryClosedPublicActions