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.

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

Allow projects to control which configuration is used by default when publishing #26534

Open
eatdrinksleepcode opened this issue Jul 13, 2022 · 4 comments
Milestone

Comments

@eatdrinksleepcode
Copy link

Problem

Recently, a new property was added to the project SDK, PublishRelease, which addresses the long-standing problem of publishing using "Debug" by default, resulting in many developers accidentally deploying a debug version of their app to production, or uploading a debug version of their library to Nuget.

However, the PublishRelease property assumes that the project has a Release configuration. While that is undoubtedly the most common case, there are many developers who customize their configurations and may not have a Release configuration, or may not want to use it as the default for publishing. The new property does not work for those cases.

For example, my project does not have a Release configuration. Instead we have multiple Release-type configurations to build for different deployment targets. We have a separate configuration for building a version of our app that is designed to run on an AWS AMI. We recently added configurations for creating a FIPS-compliant version of our app by excluding non-compliant code. And there are more.

Suggested solution

Instead of:

<PublishRelease>true</PublishRelease>

...give the developer more flexibility with:

<PublishConfiguration>Release</PublishConfiguration>

PublishConfiguration is just as easy to understand and to use as PublishRelease for the majority of projects that use the default Release configuration. But it also allows projects which have customized their configurations to make use of this functionality.

You could do both. However, you then have to define, implement, and document what happens if both are set. That scenario seems likely to be confusing for devs who are reading documentation trying to understand their options.

Additional context

I can't say for sure how my project would make use of this property if it were added. It is possible that we wouldn't use it at all; since we have multiple Release-type configurations, we might have to continue specifying them anyway. Or we might set it to our most common Release-type configuration, since that would at least be a better default than Debug. Or we might set it conditionally based on the environment.

The point is to give us the flexibility to try out different options and figure out what works for us, instead of being locked out of the feature altogether.

@nagilson
Copy link
Member

nagilson commented Jul 13, 2022

Thanks for writing this up, I think this is well thought out and I agree it would be better to provide that more granular control. I don't think we want both properties to co-exist. I prefer a minimal subset of features, why have something meant to set a default configuration to some specific value when you can just provide a value through this. So, let's see if I can kick the other feature out.

@richlander it seems pretty bad to deprecate a new feature after pushing to main, we haven't officially released it yet but we did just fork for .NET 7. Thoughts on this and the name below? It also seems to throw a wrench in the plan to make Release the default for .NET 8 if there are enough people who don't even have Release configurations. Bah.

I see how PublishConfiguration matches the other nomenclature (Publish* where * is one word) but I much prefer PublishDefaultConfiguration because it makes it clearer what it does, IMO. Maybe do a twitter poll? We should add the same for PackDefaultConfiguration

@nagilson nagilson self-assigned this Jul 13, 2022
@richlander
Copy link
Member

I think that PublishRelease is better for most people. Part of the issue with configuration is that it is case-sensitive. The boolean based property fixes that.

I can't say for sure how my project would make use of this property if it were added.

In order to build a feature, we need a strong scenario. I'm not seeing that here. So far, this looks like an idea that could be beneficial.

@nagilson
Copy link
Member

@marcpopMSFT I now agree with Rich in that there isn't a strong enough scenario for this (yet) unless a lot of people want it. It's still a cool feature, but I jumped a bit too early onto it. Unassigning but keeping it open so others can vote or chime in.

@nagilson nagilson removed their assignment Jul 18, 2022
@eatdrinksleepcode
Copy link
Author

In order to build a feature, we need a strong scenario. I'm not seeing that here. So far, this looks like an idea that could be beneficial.

I don't understand this. The scenario is:

"We don't have a Release configuration. MSBuild has supported this since the very beginning. Please don't build features that we are excluded from using by design."

Everything else is context; that statement stands on its own. Honestly given the design of configurations I would have thought that hardcoding "Release" anywhere in the project system itself would have been a non-starter.

I think that PublishRelease is better for most people.

Can you please explain why PublishConfiguration is not just as good while also supporting way more scenarios? I don't find the casing argument compelling; while case differences might present a bit of a challenge when providing configurations dynamically via CLI, we are talking here about something that will be (typically) set in the project file itself.

@marcpopMSFT marcpopMSFT removed the untriaged Request triage from a team member label Jul 29, 2022
@marcpopMSFT marcpopMSFT added this to the Backlog milestone Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants