Skip to content

Conversation

@gfcapalbo
Copy link
Contributor

@gfcapalbo gfcapalbo commented Feb 6, 2024

depends on #41 , removes dead merges and puts them in a comment at the end of the yaml.
Could not do comment in place because of PyYaml limitations.
would have to do indepth mod of the DUmper, and may not even be possible.

rualmel seems to make it possible if this solution is not okay
https://copyprogramming.com/howto/how-to-insert-a-comment-line-to-yaml-in-python-using-ruamel-yaml

currently given input:

odoo:                                                                                   
  defaults:                                                                             
    depth: ${WAFT_DEPTH_DEFAULT}                                                        
  remotes:                                                                              
    - origin: https://github.com/odoo/odoo.git  
    - origin: felafel
                                               
  target: origin ${ODOO_VERSION}                                                        
  merges:                                                                               
    - origin ${ODOO_VERSION}                                                            
                                                                                        
OCA/product-attribute:                                                                  
  defaults:                                                                             
    depth: ${WAFT_DEPTH_DEFAULT}                                                        
  remotes:                                                                              
    origin: https://github.com/OCA/product-attribute.git                                
  target: origin ${ODOO_VERSION}                                                        
  merges:                                                                               
    - origin ${ODOO_VERSION}                                                            
                                 

Where "origin felafel" is a branch that does not exist:

odoo:                                                                                   
  defaults:                                                                             
    depth: ${WAFT_DEPTH_DEFAULT}                                                        
  remotes:                                                                              
    origin: https://github.com/odoo/odoo.git                                            
  target: origin ${ODOO_VERSION}                                                        
  merges:                                                                               
    - origin ${ODOO_VERSION}                                                            
                                                                                        
OCA/product-attribute:                                                                  
  defaults:                                                                             
    depth: ${WAFT_DEPTH_DEFAULT}                                                        
  remotes:                                                                              
    origin: https://github.com/OCA/product-attribute.git                                
  target: origin ${ODOO_VERSION}                                                        
  merges:                                                                               
    - origin ${ODOO_VERSION}      

# dumped in comment section non-existing merge:  - origin felafel    from repo  odoo , remote: origin:https://github.com/odoo/odoo.git   

      

Don't know if this addition is useful , but i had an issue so i did it.

Possible problems:

  •          Resiliance: Not set up to consider connection problems. may remove branches if connection check fails
    
  •          Wrong  Remote:  in case of mispelled or non-existing remote name, it has not been tested.
    
  •          Duplicates:   HAs been tested for duplicate merges (works) , but not for duplicate remotes.
    

@gfcapalbo
Copy link
Contributor Author

gfcapalbo commented Feb 7, 2024

@thomaspaulb @Hussam-Suleiman let me know if :

  1. this function is a worthwhile one, if i should spend another hour or two hardening the three problem points i found in description.
  2. if i should spend another hour implementing in-place comments instead of "end-of-file dump" by re-implementing with ruamel and dropping YAML
  3. If i should extend this by not only identifying branches that do not exist. But also by identifying branches that have no extra commits if compared to target.
  4. if i should rename script from update_depths to a more generic name, now that it does multiple things.

@Hussam-Suleiman
Copy link
Member

In my opinion, if this script can't support all Odoo installation versions we can freeze it until the next waft version.

@gfcapalbo
Copy link
Contributor Author

In my opinion, if this script can't support all Odoo installation versions we can freeze it until the next waft version.

this message refers to #41 ? this is just an addition to that.
I respond in that MR.

@gfcapalbo gfcapalbo changed the base branch from master to master-depth-calculation February 7, 2024 12:16
@gfcapalbo gfcapalbo force-pushed the master-comment-non-existing-branches branch from 77af1ff to 289585e Compare February 7, 2024 12:19
@thomaspaulb
Copy link
Member

I think:

  • This functionality is very nice to have
  • The functionality in Master depth calculation #41 is also very nice to have
  • So, by all means, let's keep pursuing this
  • But, let's make it part of Waft 2.0, which Hussam has been working on for a while now
  • @ddejong-therp Could you make a priority out of reviewing @Hussam-Suleiman 's work in Waft 2.0?

@thomaspaulb
Copy link
Member

Meanwhile, @gfcapalbo I would let this rest, and #41 as well, until we make a decision on Waft 2.0

@thomaspaulb
Copy link
Member

PS. Also, Waft 2.0 has a separate Python3 for tools, so there, we can also use it for Odoo <=10.0

@ddejong-therp
Copy link
Contributor

Sure, last week ended up being quite filled nevertheless though.

@gfcapalbo gfcapalbo force-pushed the master-comment-non-existing-branches branch from 463c8ff to 8d8fe20 Compare March 11, 2024 11:26
@gfcapalbo
Copy link
Contributor Author

@thomaspaulb Added a connection check before looking to remove non-existing branches.
if the repo cannot be reached, nothing will be done to change the yaml.

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.

5 participants