-
Couldn't load subscription status.
- Fork 50
add dotnet provider #126
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
base: main
Are you sure you want to change the base?
add dotnet provider #126
Conversation
|
🚅 Environment railpack-pr-126 in railpack has no services deployed. |
|
@jaredLunde this looks like an awesome change and I'd love to get it merged in. Would you mind rebasing on master? Thanks! |
|
blegh, struggling to update snapshots on the hotel wifi for some reason |
|
Thanks for your work on this, excited to see this get merged! Quick question - any reason why 6 is the default version? I know it was the default version in Nixpacks but .NET 6 has been EOL since Nov 2024. .NET 8 has been the most recent LTS for almost 2 years now, which itself is soon to be replaced by .NET 10 in 2 months. Probably makes sense to have the most recent LTS be the default version? |
@danjrwalsh It's been 6 months now but if I had to guess I copy/pasted from Nixpacks. Can update. |
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.
@coffee-cup left my comments on the PR. All of them are pretty minor—I think we should merge and I can handle all of the comments in a follow-up PR.
|
|
||
| [ApiController] | ||
| [Route("[controller]")] | ||
| public class WeatherForecastController : ControllerBase |
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 doesn't look like this is used in the app entrypoint, can we remove?
| ) | ||
|
|
||
| const ( | ||
| DEFAULT_DOTNET_VERSION = "6.0.428" |
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.
Could we bump this to the latest LTS version?
| func (p *DotnetProvider) Build(ctx *generate.GenerateContext, build *generate.CommandStepBuilder) { | ||
| maps.Copy(build.Variables, p.GetEnvVars(ctx)) | ||
| build.AddCommands([]plan.Command{ | ||
| plan.NewCopyCommand("."), |
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 should use NewLocalLayer which ensures that .dockerignore is respected
|
|
||
| public int TemperatureF => 32 + (int)(TemperatureC / 0.5556); | ||
|
|
||
| public string? Summary { get; set; } |
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 don't think this file is used either
dotnet restore