Skip to content

[Workspaces] DocumentInfo never set IsGenerated value - #405

Merged
mattwar merged 1 commit into
dotnet:masterfrom
DavidKarlas:master
Feb 13, 2015
Merged

[Workspaces] DocumentInfo never set IsGenerated value#405
mattwar merged 1 commit into
dotnet:masterfrom
DavidKarlas:master

Conversation

@DavidKarlas

Copy link
Copy Markdown
Contributor

Looks like bug to me...

More baby steps :)

@pharring

Copy link
Copy Markdown
Contributor

Interesting. This change looks correct, but it brings up a few issues:

  1. I couldn't find anywhere else where IsGenerated was set, so that implies that no code is exercising this path. Which means that, as it stands today, the IsGenerated property is always false and might as well be deleted. It's referenced in exactly one place.
  2. There's no unit test exercising this property.
  3. The .With method line 93 probably needs updating to test for and propagate the isGenerated property.

@DavidKarlas Are you able to address any of those issues?

[Edited to fix hyperlinks]

@tmeschter

Copy link
Copy Markdown
Contributor

Determining whether or not a file is generated is problematic, which is probably why this field is never set. We should remove this property entirely until we actually have a design for identifying and handling generated files.

@mattwar

mattwar commented Feb 11, 2015

Copy link
Copy Markdown
Contributor

I'm surprised that this value is not getting set on DocumentInfo. It was there for a purpose. I put it there. It actually solved a problem in the past. MSBuild was changing time stamps (or regenerating) a few files it would add to the project each time the build was run (which equated to each time the project was opened) and that would throw off the version stamp calculation. So either that behavior changed, or the features that relied on the version stamp changed. Either way, the information was critical at some point.

@DavidKarlas

Copy link
Copy Markdown
Contributor Author

Maybe check also references to IsGenerated property from VisualStudio source code (http://source.roslyn.codeplex.com/ covers only Roslyn)

About 1,2,3 points...

  1. It's up to you guys, I just noticed it's never set, because I was trying to figure out what is used for :)
  2. Since I'm not sure what is used for... What unit tests can I write? To see if it's set properly?
  3. I can add that. No problem if you guys decide to keep it...

@mattwar

mattwar commented Feb 13, 2015

Copy link
Copy Markdown
Contributor

Well, it's clearly wrong in its current state and the pull request fixes it. Any further issue with whether the property should exist or not is a separate issue.

mattwar added a commit that referenced this pull request Feb 13, 2015
[Workspaces] DocumentInfo never set IsGenerated value
@mattwar
mattwar merged commit 29f5504 into dotnet:master Feb 13, 2015
dibarbet pushed a commit that referenced this pull request Aug 18, 2026
Fix package extraction in net452 builds
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.

5 participants