-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Search cmd #10956
base: master
Are you sure you want to change the base?
Search cmd #10956
Conversation
Signed-off-by: Jougan-0 <[email protected]>
… type w/signoff Signed-off-by: Jougan-0 <[email protected]>
Signed-off-by: Jougan-0 <[email protected]>
) | ||
|
||
type MeshmodelRelationshipsAPIResponse struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jougan-0 These structs should be already defined in meshkit ? // @MUzairS15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aabidsofi19 the type is in meshery/server/models/meshery_meshmodels_api_response.go
but it has the last type as entity.entity and the unmarshall was causing the issue for me .
If it is there is meshkit I am unaware of it.
|
||
availableSubcommands = []*cobra.Command{ViewRelationshipsCmd, GenerateRelationshipDocsCmd} | ||
outFormatFlag string | ||
maxRowsPerPage = 25 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the pagination logic and constants should be common to all commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aabidsofi19 I saw it was written in one of the command specifically here so I went ahead and declared for the whole relationship command.
It is 25 so i guess in a sense it is default but it should be manged from a single variable imo.
A low priority. When will the model import logs land? It's been many months now. |
I'll get to it I saw ripul had an open pr already had a look to it saw your comments too will start working on it . Is there anything before hand that I must be aware of @leecalcote You are talking about this pr? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10956 +/- ##
==========================================
- Coverage 9.01% 9.01% -0.01%
==========================================
Files 146 146
Lines 19258 19263 +5
==========================================
Hits 1736 1736
- Misses 17220 17225 +5
Partials 302 302
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Ready? |
No need to add the check for case insensitive flags. |
Notes for Reviewers
This PR fixes #9844
Signed commits