-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix the sdk version and do not name a ModelType variable "Type"
#5782
Conversation
- #5595 - enhance the `StartsWith()` check to look at what comes after
| "projects": ["src", "test/WebSites", "samples"] | ||
| "projects": ["src", "test/WebSites", "samples"], | ||
| "sdk": { | ||
| "version": "1.0.0-preview2-003154" |
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.
This is the SDK version the build downloads. Is it the correct one for the 1.0.3 milestone?
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.
It should be good for now. @JunTaoLuo, we'll need to update the CLI to the version that's shipping in the release (if any). Could you file a tracking item for this?
| string.Empty | ||
| }, | ||
| { | ||
| (Expression<Func<TestModel, int>>)(model => models[0].SelectedCategory.CategoryId), |
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.
"s[0].SelectedCategory.CategoryId" before fix
| "models[0].SelectedCategory.CategoryId" | ||
| }, | ||
| { | ||
| (Expression<Func<TestModel, string>>)(model => modelTest.Name), |
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.
"Test.Name" before fix
| "modelTest.Name" | ||
| }, | ||
| { | ||
| (Expression<Func<TestModel, Type>>)(model => modelType), |
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.
"Type" before fix
pranavkm
left a comment
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.
For that one question. Looks good otherwise.
| "projects": ["src", "test/WebSites", "samples"] | ||
| "projects": ["src", "test/WebSites", "samples"], | ||
| "sdk": { | ||
| "version": "1.0.0-preview2-003154" |
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.
It should be good for now. @JunTaoLuo, we'll need to update the CLI to the version that's shipping in the release (if any). Could you file a tracking item for this?
| { | ||
| (Expression<Func<TestModel, Type>>)(model => modelType), | ||
| "modelType" | ||
| }, |
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.
What happens if the expression uses something from a type or namespace called "Model" e.g m => MyApp.Model.Constants.CopyrightYear or some weird variant like this.
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.
We'd have to test those cases. But, this goes beyond the reported problem. So we should focus the follow-up discussion on dev.
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.
Sounds good.
|
@pranavkm this doesn't show as signed off. Do you approve? |
|
|
No description provided.