Skip to content
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

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

Merged
merged 4 commits into from
Jul 25, 2024

Conversation

Vibaswan
Copy link
Collaborator

No description provided.

@Vibaswan Vibaswan requested a review from alakendu July 18, 2024 12:41
@Vibaswan Vibaswan mentioned this pull request Jul 18, 2024
@alakendu
Copy link
Contributor

alakendu commented Jul 24, 2024

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.

@Vibaswan
Copy link
Collaborator Author

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

@Vibaswan Vibaswan merged commit 206ae33 into main Jul 25, 2024
15 checks passed
@Vibaswan Vibaswan deleted the split_gosnappi branch July 25, 2024 13:11
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.

2 participants