Bug 223881 - Style checker should warn about use of future OS versions in WK_API_AVAILABLE
Summary: Style checker should warn about use of future OS versions in WK_API_AVAILABLE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-29 10:10 PDT by Simon Fraser (smfr)
Modified: 2021-03-31 16:06 PDT (History)
6 users (show)

See Also:


Attachments
Patch v1.0 (11.84 KB, patch)
2021-03-29 17:41 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch v1.1, turn off debugging code by default (11.84 KB, patch)
2021-03-29 20:54 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers (14.69 KB, patch)
2021-03-30 10:14 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers (14.69 KB, patch)
2021-03-30 10:15 PDT, BJ Burg
no flags Details | Formatted Diff | Diff
Patch v3.0 (13.85 KB, patch)
2021-03-31 15:28 PDT, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2021-03-29 10:10:18 PDT
We need to avoid fumbles like this one: http://trac.webkit.org/changeset/273130/webkit
Comment 1 BJ Burg 2021-03-29 17:41:25 PDT
Created attachment 424606 [details]
Patch v1.0
Comment 2 Jonathan Bedard 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
Comment 3 Jonathan Bedard 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?
Comment 4 BJ Burg 2021-03-29 20:54:54 PDT
Created attachment 424614 [details]
Patch v1.1, turn off debugging code by default
Comment 5 BJ Burg 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.
Comment 6 BJ Burg 2021-03-30 10:14:46 PDT
Created attachment 424654 [details]
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers
Comment 7 BJ Burg 2021-03-30 10:15:34 PDT
Created attachment 424655 [details]
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers
Comment 8 Jonathan Bedard 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
Comment 9 BJ Burg 2021-03-31 15:28:23 PDT
Created attachment 424829 [details]
Patch v3.0

incorporated latest feedback from jbedard.
Comment 10 EWS 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].
Comment 11 Radar WebKit Bug Importer 2021-03-31 16:06:15 PDT
<rdar://problem/76076210>