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

GH2953: Allow setting MSBuild target via MSBuildSettings using a string #3721

Conversation

sbwaggoner
Copy link
Contributor

PR to address this feature request:

#2953

@dnfadmin
Copy link

dnfadmin commented Nov 29, 2021

CLA assistant check
All CLA requirements met.

@sbwaggoner
Copy link
Contributor Author

Is there something else I need to do to move this forward? It's my first contribution and I don't want to miss anything.

@augustoproiete
Copy link
Member

Thanks @sbwaggoner. You're all set. Next step is for us to review and merge or give you feedback when we have chance.

@gep13 gep13 changed the title Allow setting MSBuild target via MSBuildSettings using a string (#2953) GH2953: Allow setting MSBuild target via MSBuildSettings using a string Dec 14, 2021

private void SetTargets(string value)
{
foreach (var target in value.Split(";").Where(p => !string.IsNullOrEmpty(p)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect something like this

Suggested change
foreach (var target in value.Split(";").Where(p => !string.IsNullOrEmpty(p)))
Targets.Clear();
foreach (var target in value?.Split(';', _targetStringSplitOptions) ?? Array.Empty<string>())

so setting Target to a new value isn't keeping the old values or what do you think @augustoproiete ?

Where is something like this on top of class

#if NETCOREAPP3_1
        private static readonly StringSplitOptions _targetStringSplitOptions = StringSplitOptions.RemoveEmptyEntries;
#else
        private static readonly StringSplitOptions _targetStringSplitOptions = StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries;
#endif

@FrankRay78
Copy link
Contributor

FrankRay78 commented Aug 28, 2022

Hello @devlead, I have merged changes from sbwaggoner:feature-addition-target-initializer-msbuildsettings (which are now many commits behind) onto a local, up-to-date develop branch and intend to consider your review comments above. I've been looking for a way to contribute, perhaps this will be a good first issue for me to help get over the line.

@devlead
Copy link
Member

devlead commented Oct 4, 2022

Superceeded by #3954

@devlead devlead closed this Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants