Skip to content

Fix getAndroidHermesEnabled#1935

Merged
DmitriyKirakosyan merged 3 commits intomicrosoft:masterfrom
incleaf:patch-1
May 3, 2022
Merged

Fix getAndroidHermesEnabled#1935
DmitriyKirakosyan merged 3 commits intomicrosoft:masterfrom
incleaf:patch-1

Conversation

@incleaf
Copy link
Copy Markdown
Contributor

@incleaf incleaf commented Apr 27, 2022

There can be many cases where the line does not exactly match the enableHermes: true.
I don't think these changes would be the ultimate solution though 😓

Case 1

project.ext.react = [
        entryFile   : "index.js",
        enableHermes: true  // Enable hermers
]

Case 2

project.ext.react = [
        enableHermes: true,
        entryFile   : "index.js",
]

@AnatolyPristensky
Copy link
Copy Markdown
Contributor

Hello @incleaf and thank you for your contribution!
Looks like that your changes doesn't cover cases with multiple spaces like:
enableHermes : true,

Probably it will be better to use regex?

@incleaf
Copy link
Copy Markdown
Contributor Author

incleaf commented Apr 27, 2022

@AnatolyPristensky Agreed and applied!

image

@AnatolyPristensky
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@AnatolyPristensky
Copy link
Copy Markdown
Contributor

@incleaf ,
I see that CI is failed with the following error:

npm ERR! appcenter-cli@2.10.10 build: npm run lint && npm run compile

Could you please debug it locally and fix the error? Looks like that something goes wrong with the syntax.

@incleaf
Copy link
Copy Markdown
Contributor Author

incleaf commented Apr 28, 2022

@AnatolyPristensky Done!

@AnatolyPristensky
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@DmitriyKirakosyan
Copy link
Copy Markdown
Contributor

@incleaf I've tested your change and it works as expected. Thank you for your contribution!

@DmitriyKirakosyan DmitriyKirakosyan merged commit efe9aa4 into microsoft:master May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants