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

Re-add S.T.J.SourceGeneration.UnitTests to STJ sln #57080

Merged
merged 2 commits into from
Aug 10, 2021

Conversation

layomia
Copy link
Contributor

@layomia layomia commented Aug 9, 2021

Looks like it was dropped in #56430.

@layomia layomia added this to the 6.0.0 milestone Aug 9, 2021
@layomia layomia self-assigned this Aug 9, 2021
@ghost
Copy link

ghost commented Aug 9, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Looks like it was dropped in #56430.

Author: layomia
Assignees: layomia
Labels:

area-System.Text.Json

Milestone: 6.0.0

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 10, 2021

That's not really a sustainable solution. They will again be dropped with the next solution file format update. I will add the corresponding infra to prevent this from happening again as part of your PR. Hope that's fine.

@eiriktsarpalis
Copy link
Member

@ViktorHofer I wasn't aware that sln files were being regenerated. How can we make sure that other projects are not being dropped as well?

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 10, 2021

The slngen.template.proj is used to traverse through a leaf directory to include projects that match a certain glob pattern. We are probably missing source generators / analyzers and should add them as well. Are you aware of any other type of project that we might be missing?

EDIT: Source generators / analyzers aren't missing as those are referenced by the source project and slngen recognizes that and includes the referenced project.

Also note that #56494 will fix a few project files which are missing some reference projects.

@eiriktsarpalis
Copy link
Member

Are you aware of any other type of project that we might be missing?

Not that I know of, but I think there is risk of current or future projects being unexpectedly dropped from solution files because they happen to not match these particular glob patterns. Is there anything we could do to prevent this from happening?

@eiriktsarpalis
Copy link
Member

Build errors seem unrelated, merging away!

@eiriktsarpalis eiriktsarpalis merged commit 7249ec4 into dotnet:main Aug 10, 2021
@ViktorHofer
Copy link
Member

Not that I know of, but I think there is risk of current or future projects being unexpectedly dropped from solution files because they happen to not match these particular glob patterns. Is there anything we could do to prevent this from happening?

I would not expect a project to have any other structure than that. Our build system would need to support it otherwise explicitly. In example, the source project needs to be located under a "src" parent folder, same for ref and for test projects.

We are aiming towards a minimal diff when updating solution files which should then make any projects being dropped more visible.

@ViktorHofer
Copy link
Member

Build errors seem unrelated, merging away!

@eiriktsarpalis in future PRs please add a comment with links to issues in the PR legs. In this case it would be:

The second issue demonstrates the importance of filing issue for CI failures. @adamsitnik started working on a fix after the issue was filed.

I definitely feel the frustration about the current flakiness in the CI. This comment is meant as constructive feedback so that we can all work together on stabilizing CI and no as finger pointing.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants