-
Notifications
You must be signed in to change notification settings - Fork 418
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
Added AddProject overload that takes a project path #1071
Conversation
- To enable out of solution projects to be added trivially, added an overload of AddProject that takes the project path. - Exposed ProjectDirectory on the builder to enable resources to resolve physical paths earlier. This cleans up lots of code from having to understand if paths of fully qualified or not. Fixes #1065
/// <summary> | ||
/// Directory of the project where the app host is located. Defaults to the content root if there's no project. | ||
/// </summary> | ||
public string ProjectDirectory { get; } |
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.
@mitchdenny this cleans up A LOT and it's what your docker file PR should be using.
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.
Do you think we should call it something like AppHostDirectory instead?
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.
Yep
/azp run |
No pipelines are associated with this pull request. |
|
||
internal class ServiceMetadata(string projectPath) : IServiceMetadata | ||
{ | ||
public string AssemblyName => throw new NotImplementedException(); |
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.
Wondering whether we can ditch assembly name and assembly path now given that we don't auto name components.
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.
Looks like we don't reference them anymore. I think we initially had then because we were doing a dotnet run against the assembly itself vs. launching the project. That changed but this never got removed and the assembly name was only ever used as an input for autonaming projects which is now gone.
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.
I know I used them somewhere by newing up an instances of the generated classes to get the paths but I can't remember where. No doubt we'll remove it and then I'll find it/remember again 😄
Fixes #1065