Opened 4 months ago
Closed 4 months ago
#48447 closed defect (bug) (fixed)
Update WordPress packages with fixes targeted for 5.3 RC3
| Reported by: |  | Owned by: | |
|---|---|---|---|
| Milestone: | 5.3 | Priority: | normal | 
| Severity: | normal | Version: | |
| Component: | Editor | Keywords: | has-patch dev-feedback commit | 
| Focuses: | Cc: | 
Attachments (2)
Change History (13)
    
      
         
        
This ticket was mentioned in Slack in #core-editor by youknowriad. View the logs.
      
      
4 months ago
    
    
  
              
    
      
    #5
  
          follow-up:
    ↓ 6
    
        
          
             @
 @
            
4 months ago
        
    
  
  
    
@SergeyBiryukov The problem is that these files are automatically synced from the packages. everytime someone will perform npm install && npm run build it will be reverted.  We should fix these in Gutenberg instead.
    
      
    #6
  
        in reply to:
    ↑ 5
    
        
          
             @
 @
            
4 months ago
        
    
  
  
    
Replying to youknowriad:
The problem is that these files are automatically synced...
Right. Seems it would be good to make few "readability improvements" to latest-comments.php. Added 48447-readability-fixes.diff as an example here, will open a low-priority issue for Gutenberg too.
Generally having nested function calls with each param on its own line adds a lot of "vertical bloat" and "indentation bloat" :)
These can be avoided by making the code a bit more "wordy" but the result is quite easier to read. Also if () statements usually signify a new "block" with its own indentation. Usually better to have empty lines around.
Also, for better readability long ternary operators should probably be if () {} ... else {} blocks instead (especially when returning from them).
The six level deep nested array in register_block_core_latest_comments() is also quite "problematic" with some vertical and indentation bloat and uneven/weird white spaces but that's a bug in WPCS that needs fixing upstream (the issue has been open there for over 6 months now!).
    
      
    #7
  
    
        
          
             @
 @
            
4 months ago
        
    
  
  
    
Wooow, doing npm install:
updated 10 packages and audited 978269 packages in 22.037s found 0 vulnerabilities
Didn't realize we use nearly 1 million packages!!! :)
[46606] looks good here, +1 to merge to 5.3.
    
      
    #8
  
          follow-up:
    ↓ 9
    
        
          
             @
 @
            
4 months ago
        
    
  
  
    
These changes all look decent but IMO we should treat these files as "vendors" and not touch them at all in Core.
    
      
    #9
  
        in reply to:
    ↑ 8
    
        
          
             @
 @
            
4 months ago
        
    
  
  
    
Replying to youknowriad:
Right. Any changes would need to be made upstream (for 5.4). Opened https://github.com/WordPress/gutenberg/issues/18143.


 
                       
			     
			 
                
@youknowriad This looks good to me. Feel free to commit to
trunk. Just needs an additional sign-off before backporting!