Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Hermes to 0.7.2 for v0.64 #30561

Merged
merged 1 commit into from Dec 10, 2020
Merged

Bump Hermes to 0.7.2 for v0.64 #30561

merged 1 commit into from Dec 10, 2020

Conversation

Huxpro
Copy link
Contributor

@Huxpro Huxpro commented Dec 10, 2020

Summary

https://github.com/facebook/hermes/releases/tag/v0.7.2 is out so we can bump the pod version 馃帀

Question:
Is ~> 0.7.2 too strict? Would >= 0.7.2 be better?
Also, It doesn't look like we are restricting any version on the trunk?

Changelog

[INTERNAL] - Bump Hermes version

Test Plan

Quoting from facebook/hermes#373

I can verify 0.7.2 is working with USE_HERMES=1 bundle exec pod install --repo-update 馃帀

https://github.com/facebook/hermes/releases/tag/v0.7.2 is out so we can bump the pod version 馃帀

Question:
Is `~> 0.7.2` too strict? Would `>= 0.7.2` be better?
Also, It doesn't look like we are restricting any version on the trunk?
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Dec 10, 2020
@Huxpro Huxpro requested a review from grabbou December 10, 2020 05:23
@Huxpro Huxpro requested a review from alloy December 10, 2020 05:32
@alloy
Copy link
Contributor

alloy commented Dec 10, 2020

Is ~> 0.7.2 too strict? Would >= 0.7.2 be better?

I think keeping it is good, e.g. the JSI ABI differences thus far have meant that RN versions can't just take in any Hermes version, so locking it to patch versions equal to or higher than 0.7.2 seems good 馃憤

Also, It doesn't look like we are restricting any version on the trunk?

Can you elaborate on what you鈥檙e referring to by this?

@Huxpro
Copy link
Contributor Author

Huxpro commented Dec 10, 2020

@alloy I just have another look at https://guides.cocoapods.org/using/the-podfile.html and realize everything was good :)

so locking it to patch versions equal to or higher than 0.7.2 seems good

I misunderstood ~> 0.7.2, which actually means it only take patch releases but not minor. Yeah that is what we want.

Also, It doesn't look like we are restricting any version on the trunk?
Can you elaborate on what you鈥檙e referring to by this?

Yeah I mean we did not specify a version at the master branch, which actually means it will always take the latest, which is also what we want ;)

@Huxpro Huxpro merged commit 52129b2 into 0.64-stable Dec 10, 2020
@grabbou
Copy link
Contributor

grabbou commented Dec 10, 2020

Just curious if this was intentional or a mistake that this PR was opened against release branch and didn't land on master? In such case, it may need to be synced once again.

@Huxpro
Copy link
Contributor Author

Huxpro commented Dec 11, 2020

@grabbou it was intentional since I observed that they are already not synced: we specified 0.7.1 at the 0.64 branch but did not specify a version at the master branch.

But quoting my later realization:

Yeah I mean we did not specify a version at the master branch, which actually means it will always take the latest

which seems to be what we want?

@yungsters yungsters deleted the bump-hermes-to-0.7.2 branch July 15, 2021 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants