Skip to content

Conversation

Gregoirevda
Copy link
Contributor

ruby function install_modules_dependencies was added in 0.71.0-rc.3 facebook/react-native@82e9c6a

ruby function `install_modules_dependencies` was added in 0.71.0-rc.3
facebook/react-native@82e9c6a
@netlify
Copy link

netlify bot commented Dec 5, 2022

Deploy Preview for react-native ready!

Name Link
🔨 Latest commit 90510b3
🔍 Latest deploy log https://app.netlify.com/sites/react-native/deploys/638e7471bbda58000915db76
😎 Deploy Preview https://deploy-preview-3455--react-native.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cortinico
Copy link
Contributor

I'm not sure I follow why you want to make this change.
The New Architecture was introduced actually in 0.68+ so the current wording is correct. Could you clarify what's the intention of this PR?

@Gregoirevda
Copy link
Contributor Author

@cortinico The podspec file uses the ruby function install_modules_dependencies which isn't available in latest react-native version (the function was only added in the commit I've referenced which is available under the npmjs next tag)

@cortinico
Copy link
Contributor

@cortinico The podspec file uses the ruby function install_modules_dependencies which isn't available in latest react-native version (the function was only added in the commit I've referenced which is available under the npmjs next tag)

Yup, but you're updating the documentation for next so for when 0.71.0 will be out. You should not reference rc3 and just mention that the docs assumes you're on 0.71.0 similarly to what I did here: #3453

Copy link
Contributor

@NickGerleman NickGerleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should generally avoid adding version numbers in documents like this since they age poorly. E.g. the document will stay the same once 0.72 comes out.

The documents are versioned though, so the contents of the current page will only be showed for the 0.71 version of the website, and not the versions before it.

@cortinico
Copy link
Contributor

We should generally avoid adding version numbers in documents like this since they age poorly. E.g. the document will stay the same once 0.72 comes out.

True. Yet for the New Architecture setup, the version number is sort of crucial IMHO as we're keep on iterating on the migration path.

@cipolleschi
Copy link
Contributor

Hi @Gregoirevda, thank you a lot for the PR. However, I don't think we have to merge this.

The podspec file you mention is in the Fabric Components. The Fabric Component page has this lines:

The following section guides you through the creation of a Fabric Native Component, step-by-step, targeting the latest version of React Native
By the time we ship this documentation page, 0.71 will be the latest version of React Native, so the documentation is correct.

You are suggesting to change the [use-app-template.md](https://github.com/facebook/react-native-website/pull/3455/files#diff-ab38314fcc7012e596ef41d1c26754017d05a18803f41a684fc4f5a7f50a1db8) page which is a page where we are helping people to get started with their apps

  1. Apps have nothing to spare with the podspecs, which are for libraries. It would be confusing for the user to look at the documentation and see a specific version of React Native (especially an RC) just to discover that it is due to the lib configurations.
  2. By the time we release the change, 0.71 would be the lastest stable and your changes will be outdated, requiring another update.

What do you think?

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above

@Gregoirevda
Copy link
Contributor Author

Makes sense to not include version numbers, especially RC. Just a matter of time before install_modules_dependencies is included in latest

@Gregoirevda Gregoirevda deleted the patch-2 branch December 22, 2022 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants