-
Notifications
You must be signed in to change notification settings - Fork 101
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
Move MeshModel to Model. #468
base: master
Are you sure you want to change the base?
Conversation
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack. |
@@ -42,7 +42,7 @@ type ComponentDefinition struct { | |||
UpdatedAt time.Time `json:"-"` | |||
} | |||
type ComponentDefinitionDB struct { | |||
ID uuid.UUID `json:"id"` | |||
ID uuid.UUID `json:"componentDefinitionId"` |
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.
@MUzairS15 @leecalcote To fix the golangci-lint issue, I renamed the Json tag. Will it have an impact on its dependencies?
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.
@Yashsharma1911 Could confirm.
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.
@Yashsharma1911 may be slow to response, therefore I revert this unpredictable modification so that we can keep going. // @MUzairS15 @leecalcote
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.
@RipulHandoo, I wonder if you might be familiar here.
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.
@leecalcote @MUzairS15 Can we go ahead, and I can create another issue mentioning @Yashsharma1911 @RipulHandoo to track it?
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 issue created: #469
models/model/core/v1alpha1/models.go
Outdated
@@ -60,7 +60,7 @@ type Model struct { | |||
} | |||
|
|||
type ModelDB struct { | |||
ID uuid.UUID `json:"id"` | |||
ID uuid.UUID `json:"modelDBId"` |
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.
Ditto.
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.
Note: This will be a breaking change.
cb3833a
to
b13db22
Compare
@leecalcote Do you have any comments? |
@@ -35,7 +35,7 @@ type ComponentParameter struct { | |||
Description *string `json:"description,omitempty"` | |||
} | |||
|
|||
const MesheryAnnotationPrefix = "design.meshmodel.io" | |||
const MesheryAnnotationPrefix = "design.model.io" |
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.
@MUzairS15 this is to be design.meshery.io
, right?
Thank you for asking. I only have two:
|
IMO, the dependencies sequence should be meshkit <- meshery-adapter-library <- Meshery components (Adapters, Mesheryctl... ) In order to make the transformation seamlessly, we can create a release of meshkit, then validate the meshery-adapter-library can work if update the meshkit version. Then we can create the release of meshery-adapter-library to validate other Meshery components base on that. This is a process that requires care and patience. @MUzairS15 Please correct me if I missed something. |
Here's a related / similar chore - meshery/meshery#9969 |
This is making sense to me, @gyohuangxin. @MUzairS15, sound about right? |
@gyohuangxin Yes, its correct. While adapter-library and server can be done in parallel. |
While I propose the change for this to be done, when the first iteration of the refactoring from I'll create a issue to track this item. |
@gyohuangxin Since Will you first start the migration in Meshery Server by using this branch as local go.mod reference? |
@MUzairS15 Got it. Will work on that. |
Signed-off-by: Huang Xin <[email protected]>
Signed-off-by: Xin Huang <[email protected]>
Signed-off-by: Xin Huang <[email protected]>
Signed-off-by: Mohd Uzair <[email protected]>
Signed-off-by: Mohd Uzair <[email protected]>
Signed-off-by: Mohd Uzair <[email protected]>
Signed-off-by: Mohd Uzair <[email protected]>
Uh-oh. A merge conflict popped up. |
b13db22
to
c753c2d
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Description
This PR fixes #467
Notes for Reviewers
Signed commits