Bug 223881 – Style checker should warn about use of future OS versions in WK_API_AVAILABLE

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Simon Fraser (smfr)]

We need to avoid fumbles like this one: http://trac.webkit.org/changeset/273130/webkit

[Blaze Burg]

Created attachment 424606 [details]
Patch v1.0

[Jonathan Bedard]

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 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]

Created attachment 424614 [details]
Patch v1.1, turn off debugging code by default

[Blaze Burg]

(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]

Created attachment 424654 [details]
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers

[Blaze Burg]

Created attachment 424655 [details]
Patch v2.1 - use Version / VersionNameMap to parse and compare version numbers

[Jonathan Bedard]

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]

Created attachment 424829 [details]
Patch v3.0

incorporated latest feedback from jbedard.

[EWS]

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]

<rdar://problem/76076210>