- 
                Notifications
    
You must be signed in to change notification settings  - Fork 24.9k
 
Refactor hermes-utils.rb #38963
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Closed
      
      
    
                
     Closed
            
            Refactor hermes-utils.rb #38963
      
        
          +148
        
        
          −86
        
        
          
        
      
    
  
Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    | 
           This pull request was exported from Phabricator. Differential Revision: D48161239  | 
    
| 
           This pull request was exported from Phabricator. Differential Revision: D48161239  | 
    
    
  dmytrorykun 
      added a commit
        to dmytrorykun/react-native
      that referenced
      this pull request
    
      Aug 11, 2023 
    
    
      
  
    
      
    
  
Summary: Pull Request resolved: facebook#38963 This diff refactors `hermes-utils.rb` in the following way: - Explicitly define all the scenarios how `hermes-engine.podspec` can consume its source. - Try to be explicit and verbose about when those scenarios take place. - Split `compute_hermes_source` into two functions: - `hermes_source_type` that determines the podspec source type. - `podspec_source` that builds the podspec source based on the source type provided. Also `hermes-engine.podspec` now uses source type returned by `hermes_source_type` instead of derived values `:git` and `:http`, which were not descriptive and granular enough. This refactoring should make adding new cases and altering existing ones easier. Conditions, precedence and how to act for each scenario is much more explicit. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D48161239 fbshipit-source-id: fa2d29ad337d2faba58bce618482c8f7de607429
08ab76d    to
    0671d2e      
    Compare
  
    
          
 Base commit: d655d44  | 
    
    
  dmytrorykun 
      added a commit
        to dmytrorykun/react-native
      that referenced
      this pull request
    
      Aug 21, 2023 
    
    
      
  
    
      
    
  
Summary: Pull Request resolved: facebook#38963 This diff refactors `hermes-utils.rb` in the following way: - Explicitly define all the scenarios how `hermes-engine.podspec` can consume its source. - Try to be explicit and verbose about when those scenarios take place. - Split `compute_hermes_source` into two functions: - `hermes_source_type` that determines the podspec source type. - `podspec_source` that builds the podspec source based on the source type provided. Also `hermes-engine.podspec` now uses source type returned by `hermes_source_type` instead of derived values `:git` and `:http`, which were not descriptive and granular enough. This refactoring should make adding new cases and altering existing ones easier. Conditions, precedence and how to act for each scenario is much more explicit. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D48161239 fbshipit-source-id: c180d6d40ebb71302701c584034e6c781ef48fb6
0671d2e    to
    d7d8faf      
    Compare
  
    | 
           This pull request was exported from Phabricator. Differential Revision: D48161239  | 
    
    
  dmytrorykun 
      added a commit
        to dmytrorykun/react-native
      that referenced
      this pull request
    
      Aug 23, 2023 
    
    
      
  
    
      
    
  
Summary: Pull Request resolved: facebook#38963 This diff refactors `hermes-utils.rb` in the following way: - Explicitly define all the scenarios how `hermes-engine.podspec` can consume its source. - Try to be explicit and verbose about when those scenarios take place. - Split `compute_hermes_source` into two functions: - `hermes_source_type` that determines the podspec source type. - `podspec_source` that builds the podspec source based on the source type provided. Also `hermes-engine.podspec` now uses source type returned by `hermes_source_type` instead of derived values `:git` and `:http`, which were not descriptive and granular enough. This refactoring should make adding new cases and altering existing ones easier. Conditions, precedence and how to act for each scenario is much more explicit. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D48161239 fbshipit-source-id: 14e96d55db0dad6a66dc864cb27484562d184f44
d7d8faf    to
    ab7fc41      
    Compare
  
    
    
  dmytrorykun 
      added a commit
        to dmytrorykun/react-native
      that referenced
      this pull request
    
      Aug 23, 2023 
    
    
      
  
    
      
    
  
Summary: Pull Request resolved: facebook#38963 This diff refactors `hermes-utils.rb` in the following way: - Explicitly define all the scenarios how `hermes-engine.podspec` can consume its source. - Try to be explicit and verbose about when those scenarios take place. - Split `compute_hermes_source` into two functions: - `hermes_source_type` that determines the podspec source type. - `podspec_source` that builds the podspec source based on the source type provided. Also `hermes-engine.podspec` now uses source type returned by `hermes_source_type` instead of derived values `:git` and `:http`, which were not descriptive and granular enough. This refactoring should make adding new cases and altering existing ones easier. Conditions, precedence and how to act for each scenario is much more explicit. Changelog: [Internal] Differential Revision: https://internalfb.com/D48161239 fbshipit-source-id: 02782977c92e4db1fe1503b71120389a244cf36e
Summary: Pull Request resolved: facebook#38963 This diff refactors `hermes-utils.rb` in the following way: - Explicitly define all the scenarios how `hermes-engine.podspec` can consume its source. - Try to be explicit and verbose about when those scenarios take place. - Split `compute_hermes_source` into two functions: - `hermes_source_type` that determines the podspec source type. - `podspec_source` that builds the podspec source based on the source type provided. Also `hermes-engine.podspec` now uses source type returned by `hermes_source_type` instead of derived values `:git` and `:http`, which were not descriptive and granular enough. This refactoring should make adding new cases and altering existing ones easier. Conditions, precedence and how to act for each scenario is much more explicit. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D48161239 fbshipit-source-id: b3831e3e13edcb0bc775d2c7ad606dc536325e39
| 
           This pull request was exported from Phabricator. Differential Revision: D48161239  | 
    
ab7fc41    to
    9655639      
    Compare
  
    | 
           This pull request has been merged in 4b664fc.  | 
    
    
  facebook-github-bot 
      pushed a commit
      that referenced
      this pull request
    
      Sep 1, 2023 
    
    
      
  
    
      
    
  
Summary: #38963 introduced a regression that breaks downloading hermes tarball for nightlies. It fails with the following error when running `pod install`: ``` Fetching podspec for `hermes-engine` from `../../../node_modules/react-native/sdks/hermes-engine/hermes-engine.podspec` [!] Failed to load 'hermes-engine' podspec: [!] Invalid `hermes-engine.podspec` file: undefined local variable or method `react_native_path' for Pod:Module Did you mean? react_native_post_install. ``` The root cause is that the `download_nightly_hermes` method was just not implemented. ## Changelog: [IOS] [FIXED] - Fixed downloading hermes tarball for nightlies Pull Request resolved: #39243 Test Plan: After applying these changes, `pod install` in my project no longer fails and downloads the tarball correctly. Reviewed By: dmytrorykun Differential Revision: D48897925 Pulled By: cipolleschi fbshipit-source-id: a504f01b739862bd092f22bca1f240f6df3a6df8
  
    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. 
  
    fb-exported
  
    Merged
  This PR has been merged. 
  
    p: Facebook
  Partner: Facebook 
  
    Partner
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
Summary:
This diff refactors
hermes-utils.rbin the following way:hermes-engine.podspeccan consume its source.compute_hermes_sourceinto two functions:hermes_source_typethat determines the podspec source type.podspec_sourcethat builds the podspec source based on the source type provided.Also
hermes-engine.podspecnow uses source type returned byhermes_source_typeinstead of derived values:gitand:http, which were not descriptive and granular enough.This refactoring should make adding new cases and altering existing ones easier. Conditions, precedence and how to act for each scenario is much more explicit.
Changelog: [Internal]
Reviewed By: cipolleschi
Differential Revision: D48161239