Making JSI_EXPORT macro definition conditional #22561
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Generated by 🚫 dangerJS |
|
Can you add some more context to your summary for how this will be used? In some ways you are asking for this to become part of our public API so that other projects can define it differently. It would be good to know the use cases for that in a bit more detail. |
|
This isn't changing it to be a public API. Its already marked as JSI_EXPORT. This change just makes the code not override an existing definition for JSI_EXPORT, say if you wanted to #define it to be __dllexport so that you can build this file using MSVC. |
|
Yep. Andrew has perfectly explained the motivation behind the change. Essentially we want the macro to be redefinable. |
|
cc @mhorowitz. Can you review this change? |
There was a problem hiding this comment.
Just talked with @mhorowitz and he verified that this change is fine. @mganandraj, can you sign the CLA so that I can land this internally?
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
@RSNara has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Anandraj Govindan merged commit 1d0404c into |
Summary: This change enables defining the macro at a more global and central location. And thereby allowing us to building this file using MSVC. Pull Request resolved: facebook#22561 Reviewed By: fkgozali Differential Revision: D13529762 Pulled By: RSNara fbshipit-source-id: c78abb26f47aba25d50305c5bc0b23b6568f1243
This change enables defining the macro at a more global and central location. And thereby allowing us to building this file using MSVC.