Page MenuHomeFreeBSD
Differential D18913

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

Authored by aryeeteygerald_rogers.com on Jan 21 2019, 5:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 13, 5:17 PM
Unknown Object (File)
Thu, Nov 6, 2:57 AM
Unknown Object (File)
Thu, Nov 6, 1:44 AM
Unknown Object (File)
Wed, Nov 5, 5:16 PM
Unknown Object (File)
Wed, Nov 5, 9:15 AM
Unknown Object (File)
Fri, Oct 31, 5:40 AM
Unknown Object (File)
Sat, Oct 25, 3:32 AM
Unknown Object (File)
Wed, Oct 22, 1:01 AM
View All 97 Files
Subscribers
cperciva
delphij
jilles

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 22073
Build 21300: 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
delphij requested changes to this revision.Jan 22 2019, 5:26 AM

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
aryeeteygerald_rogers.com updated this revision to Diff 53086.Jan 22 2019, 3:43 PM
  • 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
aryeeteygerald_rogers.com updated this revision to Diff 53087.Jan 22 2019, 3:46 PM

Remove unnecessary diff junk.

aryeeteygerald_rogers.com marked an inline comment as done.Jan 22 2019, 5:11 PM
delphij accepted this revision.Jan 24 2019, 5:51 AM

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
emaste closed this revision.Jan 24 2019, 6:28 PM

Forgot the review link; committed in rS343407

Revision Contents
Changeset List

PathSize
1 line
contrib/
libarchive/
libarchive/
5 lines
5 lines
usr.bin/
ar/
6 lines
3 lines
60 lines
usr.sbin/
freebsd-update/
74 lines

Diff 53086

View Options

.gitignore

Loading...
View Options

contrib/libarchive/libarchive/archive_read_support_format_ar.c

Loading...
View Options

contrib/libarchive/libarchive/archive_write_set_format_ar.c

Loading...
View Options

usr.bin/ar/ar.h

Loading...
View Options

usr.bin/ar/read.c

Loading...
View Options

usr.bin/ar/write.c

Loading...
View Options

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

Loading...