google / macops

Utilities, tools, and scripts for managing and tracking a fleet of Macintoshes in a corporate environment
Apache License 2.0
819 stars 86 forks source link

Require minor version in Deprecation Notifier #68

Closed ChefAustin closed 6 years ago

ChefAustin commented 6 years ago

PR to fix #67

Tested on macOS 10.13.2 using 10.13.0, 10.13.1, 10.13.2, 10.13.3 as expectedVersion; all passed.

googlebot commented 6 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


russellhancox commented 6 years ago

Hi Austin,

This change would fail to show properly on machines running 10.13, as the SystemVersion.plist contains just "10.13". Could you change it to handle the case where systemVersionArray and/or expectedVersionArray only have 2 elements?

E.g. all of these states?

expectedVersion systemVersion state
10.12.6 10.12.5 display
10.13 10.12.6 display
10.13 10.13 quit
10.13 10.13.2 quit
10.13.2 10.13 display
10.13.2 10.13.1 display
10.13.2 10.13.2 quit
ChefAustin commented 6 years ago

Valid points. @russellhancox. I'll poke at it a bit more. Thanks.

googlebot commented 6 years ago

CLAs look good, thanks!

googlebot commented 6 years ago

CLAs look good, thanks!

russellhancox commented 6 years ago

Thanks for the contribution!