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.

Sign up for GitHub

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

Jump to bottom

Make referencing Node/Npm projects consistent with python and dotnet #5020

Open
maddymontaquila opened this issue Jul 23, 2024 · 3 comments · May be fixed by #5759
Open

Make referencing Node/Npm projects consistent with python and dotnet #5020

maddymontaquila opened this issue Jul 23, 2024 · 3 comments · May be fixed by #5759
Labels
area-integrations Issues pertaining to Aspire Integrations packages feature A single user-facing feature. Can be grouped under an epic.
Milestone
9.0

Comments

@maddymontaquila
Copy link
Member

Background and Motivation

When adding a Python or Dotnet project, the API is builder.AddProject / builder.AddPythonProject - Npm/Node should probably be consistent and not AddNodeApp etc. Also I'm sure there is a reason they are two separate APIs, but... feels like we should have builder.AddJSProject("node") or something and abstract that

Proposed API

no

Usage Examples

builder.AddNodeProject("pls");
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jul 23, 2024
@maddymontaquila maddymontaquila changed the title Make Node/Npm consistent with python and dotnet Make referencing Node/Npm projects consistent with python and dotnet Jul 23, 2024
@aaronpowell
Copy link
Contributor

I find the naming a little confusing because the NodeAppResource and PythonProjectResource are of type ExecutableResource not ProjectResource, which means that from a manifest perspective they are treated differently (resulting in a different hosting model).

I would argue that it should be AddPythonApp to align with the current Node style and thus have a point of difference that they will be handled differently in the manifest for hosting.

@maddymontaquila
Copy link
Member Author

after talking with @DamianEdwards I agree with you @aaronpowell !!! It should be "AddNodeApp" and "AddPythonApp" and we should reserve "Add<>Project" for things with a .csproj file!

@DamianEdwards DamianEdwards added this to the 8.2 milestone Aug 24, 2024
@DamianEdwards
Copy link
Member

We should add the new method (AddPythonApp) and obsolete the existing one in 8.2 and, remove the old one in 9.0

@davidfowl davidfowl added area-integrations Issues pertaining to Aspire Integrations packages and removed area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication labels Sep 7, 2024
@davidfowl davidfowl modified the milestones: 8.2, 9.0 Sep 7, 2024
@davidfowl davidfowl added the feature A single user-facing feature. Can be grouped under an epic. label Sep 7, 2024
@maddymontaquila maddymontaquila linked a pull request Sep 18, 2024 that will close this issue
Open
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-integrations Issues pertaining to Aspire Integrations packages feature A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Milestone
9.0
Development

Successfully merging a pull request may close this issue.

Update "PythonProjectResource" to "PythonAppResource" dotnet/aspire
4 participants
@davidfowl @DamianEdwards @aaronpowell @maddymontaquila