Bug 223881

Summary: Style checker should warn about use of future OS versions in WK_API_AVAILABLE
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Tools / TestsAssignee: Blaze Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bburg, ews-watchlist, glenn, jbedard, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 301475    
Attachments:
Description Flags
Patch v1.0
none
Patch v1.1, turn off debugging code by default
none
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers
none
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers
none
Patch v3.0 none

Simon Fraser (smfr)
Reported 2021-03-29 10:10:18 PDT
We need to avoid fumbles like this one: http://trac.webkit.org/changeset/273130/webkit
Attachments
Patch v1.0 (11.84 KB, patch)
2021-03-29 17:41 PDT, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Patch v1.1, turn off debugging code by default (11.84 KB, patch)
2021-03-29 20:54 PDT, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers (14.69 KB, patch)
2021-03-30 10:14 PDT, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers (14.69 KB, patch)
2021-03-30 10:15 PDT, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Patch v3.0 (13.85 KB, patch)
2021-03-31 15:28 PDT, Blaze Burg 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Blaze Burg
Comment 1 2021-03-29 17:41:25 PDT
Created attachment 424606 [details] Patch v1.0
Jonathan Bedard
Comment 2 2021-03-29 17:52:30 PDT
Comment on attachment 424606 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=424606&action=review > Tools/Scripts/webkitpy/style/checkers/cpp.py:3309 > + MAX_ALLOWED_MACOS_VERSION = 11.0 We have these hard-coded in Tools/Scripts/webkitpy/common/version_name_map.py, we're trying to centralize versions for this sort of thing in that file
Jonathan Bedard
Comment 3 2021-03-29 17:54:32 PDT
Comment on attachment 424606 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=424606&action=review > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:51 > +# Enable this if you need to see the full diff when debugging failures. Did you intend to include this as part of the change?
Blaze Burg
Comment 4 2021-03-29 20:54:54 PDT
Created attachment 424614 [details] Patch v1.1, turn off debugging code by default
Blaze Burg
Comment 5 2021-03-29 20:55:17 PDT
(In reply to Jonathan Bedard from comment #2) > Comment on attachment 424606 [details] > Patch v1.0 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=424606&action=review > > > Tools/Scripts/webkitpy/style/checkers/cpp.py:3309 > > + MAX_ALLOWED_MACOS_VERSION = 11.0 > > We have these hard-coded in > Tools/Scripts/webkitpy/common/version_name_map.py, we're trying to > centralize versions for this sort of thing in that file OK, will take a peep.
Blaze Burg
Comment 6 2021-03-30 10:14:46 PDT
Created attachment 424654 [details] Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers
Blaze Burg
Comment 7 2021-03-30 10:15:34 PDT
Created attachment 424655 [details] Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers
Jonathan Bedard
Comment 8 2021-03-30 11:19:51 PDT
Comment on attachment 424655 [details] Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers View in context: https://bugs.webkit.org/attachment.cgi?id=424655&action=review > Tools/Scripts/webkitpy/style/checkers/cpp.py:3314 > + if platform_name == "macos": I think we should make this a loop through a dictionary so we can support other platforms (namely watchOS) and de-duplicate code. Looking through the differences between iOS and macOS, should reduce the size of this function too > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:51 > +# Enable this if you need to see the full diff when debugging failures. I think we should put this in a different change
Blaze Burg
Comment 9 2021-03-31 15:28:23 PDT
Created attachment 424829 [details] Patch v3.0 incorporated latest feedback from jbedard.
EWS
Comment 10 2021-03-31 16:05:07 PDT
Committed r275313: <https://commits.webkit.org/r275313> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424829 [details].
Radar WebKit Bug Importer
Comment 11 2021-03-31 16:06:15 PDT
<rdar://problem/76076210>
Note You need to log in before you can comment on or make changes to this bug.