open-traffic-generator / openapiart

OpenAPI artifact generator
MIT License
6 stars 4 forks source link

Split gosnappi files into smaller files for better pars ability #469

Closed Vibaswan closed 1 month ago

alakendu commented 1 month ago

It is quite difficult to predict ultimate collateral, just looking into the changes (https://github.com/open-traffic-generator/openapiart/pull/469). Overall, it looks like we are splitting files according to the objects. So, I have put these generic review comments:

  1. We are end-up with huge numbers of files. Thing is really ugly for pattern_flow_*.go. (https://github.com/open-traffic-generator/snappi/tree/split_gosnappi/gosnappi). I do not have ready solution, but what about if we use some pattern matching using object name also (say if we match pattern_flow_rsvp_path* then 128 files will club into one file) . At the same time we already got review from google, so I don’t want to drag it farther.
  2. We must build controller and run entire CI using this snappi (with enable split flag) to make sure split not left any structure.
  3. Generate a snappi build without split flag. There gosnappi should exact match with gosnappi of main branch.
Vibaswan commented 1 month ago

It is quite difficult to predict ultimate collateral, just looking into the changes (#469). Overall, it looks like we are splitting files according to the objects. So, I have put these generic review comments:

  1. We are end-up with huge numbers of files. Thing is really ugly for pattern_flow_*.go. (https://github.com/open-traffic-generator/snappi/tree/split_gosnappi/gosnappi). I do not have ready solution, but what about if we use some pattern matching using object name also (say if we match pattern_flow_rsvp_path* then 128 files will club into one file) . At the same time we already got review from google, so I don’t want to drag it farther.
  2. We must build controller and run entire CI using this snappi (with enable split flag) to make sure split not left any structure.
  3. Generate a snappi build without split flag. There gosnappi should exact match with gosnappi of main branch.

we have checked for points 2 and 3