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

Suggestion to parse solution when projects have similar names #5367

Merged
merged 16 commits into from
Aug 14, 2020

Conversation

joseotavioq
Copy link
Contributor

Created a fix to detect collision caused by a unique project name's normalization, reported on issue #3019

Copy link
Contributor

@Nirmal4G Nirmal4G left a comment

Choose a reason for hiding this comment

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

Naming suggestions. What do you think?

src/Build/Construction/Solution/ProjectInSolution.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/ProjectInSolution.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/SolutionFile.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/ProjectInSolution.cs Outdated Show resolved Hide resolved
@Forgind
Copy link
Member

Forgind commented May 22, 2020

What's wrong with this:
Forgind@13ae999
?

@joseotavioq
Copy link
Contributor Author

What's wrong with this:
Forgind@13ae999
?

As far as I know, the unique name is based on the project name, not the file name.

joseotavioq and others added 3 commits May 26, 2020 10:01
Co-authored-by: Nirmal Guru <Nirmal4G@gmail.com>
Co-authored-by: Nirmal Guru <Nirmal4G@gmail.com>
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Thanks @joseotavioq! This is looking pretty good. Would you like to mark it ready for review?

src/Build.UnitTests/Construction/SolutionFile_Tests.cs Outdated Show resolved Hide resolved
src/Build.UnitTests/Construction/SolutionFile_Tests.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/ProjectInSolution.cs Outdated Show resolved Hide resolved
@joseotavioq joseotavioq marked this pull request as ready for review July 24, 2020 20:33
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

I think this works—just one part I wasn't sure of.

src/Build.UnitTests/Construction/SolutionFile_Tests.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/SolutionFile.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/SolutionFile.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/SolutionFile.cs Outdated Show resolved Hide resolved
src/Build/Construction/Solution/SolutionFile.cs Outdated Show resolved Hide resolved
joseotavioq and others added 4 commits August 12, 2020 19:02
Changing comments

Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
@Forgind Forgind merged commit a55ce4f into dotnet:master Aug 14, 2020
@Forgind
Copy link
Member

Forgind commented Aug 14, 2020

Thanks for your work here!

@joseotavioq joseotavioq deleted the fix_sln_parser_proj_similar_names branch August 17, 2020 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants