Make SurfaceManager const correct#48485

Closed

christophpurrer wants to merge 1 commit into

facebook:mainfrom

christophpurrer:export-D67820439

Contributor

christophpurrer commented Jan 4, 2025

Summary:
[Changelog] [Internal] - Make SurfaceManager const correct

This marks methods which don't modify member props as const and others as non const.

The current API signature is misleading as const methods do alter mutable members

Differential Revision: D67820439

facebook-github-bot added CLA Signed p: Facebook Partner labels

Contributor

facebook-github-bot commented Jan 4, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

facebook-github-bot added the fb-exported label

christophpurrer force-pushed the export-D67820439 branch from ddbbde3 to a2273d9 Compare

January 4, 2025 08:56

christophpurrer force-pushed the export-D67820439 branch from a2273d9 to 38cd580 Compare

January 4, 2025 08:56

Contributor

facebook-github-bot commented Jan 4, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

1 similar comment

Contributor

facebook-github-bot commented Jan 4, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

christophpurrer force-pushed the export-D67820439 branch from 38cd580 to e9cf7f7 Compare

January 4, 2025 18:16

Contributor

facebook-github-bot commented Jan 4, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

christophpurrer force-pushed the export-D67820439 branch from e9cf7f7 to c86a5b6 Compare

January 4, 2025 18:17

Contributor

facebook-github-bot commented Jan 4, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

Contributor

shergin commented Jan 5, 2025

@christophpurrer I don't know which problem you are solving, but I suspect this change is not correct.
I believe, what you are calling misleading here is a just standard C++ approach for designating thread-safe methods.
I think there is a classic youtube talk by Herb Sutter about that where he's describing that in details.

christophpurrer force-pushed the export-D67820439 branch from c86a5b6 to 8572ca1 Compare

January 6, 2025 17:19

Contributor

facebook-github-bot commented Jan 6, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

christophpurrer force-pushed the export-D67820439 branch from 8572ca1 to f58b490 Compare

January 6, 2025 20:22


          Make SurfaceManager const correct (facebook#48485)

6ff0bf3

Summary:

[Changelog] [Internal] - Make SurfaceManager const correct

This marks methods which don't modify member props as `const` and others as non `const`.

The current API signature is misleading as `const` methods do alter `mutable` members

Reviewed By: rshest

Differential Revision: D67820439

Contributor

facebook-github-bot commented Jan 6, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

christophpurrer force-pushed the export-D67820439 branch from f58b490 to 6ff0bf3 Compare

January 6, 2025 20:22

Contributor

facebook-github-bot commented Jan 6, 2025

This pull request was exported from Phabricator. Differential Revision: D67820439

christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request


          Make SurfaceManager const correct (facebook#48485)

f52c685

Summary:

[Changelog] [Internal] - Make SurfaceManager const correct

This marks methods which don't modify member props as `const` and others as non `const`.

The current API signature is misleading as `const` methods do alter `mutable` members

Reviewed By: rshest

Differential Revision: D67820439

christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request


          [skip ci] Make SurfaceManager const correct (facebook#48485)

3b1d14b

Summary:

[Changelog] [Internal] - Make SurfaceManager const correct

This marks methods which don't modify member props as `const` and others as non `const`.

The current API signature is misleading as `const` methods do alter `mutable` members

Reviewed By: rshest

Differential Revision: D67820439

facebook-github-bot closed this in

c855251

facebook-github-bot added the Merged label

Contributor

facebook-github-bot commented Jan 6, 2025

This pull request has been merged in c855251.

Collaborator

react-native-bot commented Jan 6, 2025

This pull request was successfully merged by @christophpurrer in c855251

^{When will my fix make it into a release? | How to file a pick request?}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed fb-exported Merged p: Facebook Partner