Introduce IncludeSharedGuardSource property

[Kielek]

Changes

Introduce IncludeSharedGuardSource property to define source file only once.

For significant contributions please make sure you have completed the following items:

• [ ] Appropriate CHANGELOG.md updated for non-trivial changes
• [ ] Design discussion issue #
• [ ] Changes in public API reviewed

[codecov]

Codecov Report

Merging #1430 (785a2ad) into main (71655ce) will decrease coverage by 0.99%.
Report is 52 commits behind head on main.
The diff coverage is 62.98%.

@@            Coverage Diff             @@
##             main    #1430      +/-   ##
==========================================
- Coverage   73.91%   72.92%   -0.99%     
==========================================
  Files         267      262       -5     
  Lines        9615     9682      +67     
==========================================
- Hits         7107     7061      -46     
- Misses       2508     2621     +113     

Flag	Coverage Δ
unittests-Exporter.Geneva	57.92% <27.45%> (?)
unittests-Exporter.OneCollector	89.71% <ø> (?)
unittests-Extensions	81.73% <ø> (?)
unittests-Instrumentation.AspNet	72.91% <94.44%> (?)
unittests-Instrumentation.EventCounters	75.92% <ø> (?)
unittests-Instrumentation.Owin	85.71% <100.00%> (?)
unittests-Instrumentation.Process	100.00% <ø> (?)
unittests-Instrumentation.Runtime	100.00% <ø> (?)
unittests-Instrumentation.StackExchangeRedis	71.37% <6.45%> (?)
unittests-Instrumentation.Wcf	78.47% <81.59%> (?)
unittests-PersistentStorage	58.80% <ø> (?)
unittests-ResourceDetectors.Azure	80.73% <50.00%> (?)
unittests-Solution	78.94% <39.02%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...orter.Geneva/MsgPackExporter/MsgPackLogExporter.cs	95.10% <100.00%> (+1.76%)	⬆️
...ter.Geneva/MsgPackExporter/MsgPackTraceExporter.cs	94.52% <100.00%> (+2.10%)	⬆️
...ry.Exporter.InfluxDB/InfluxDBExporterExtensions.cs	100.00% <100.00%> (ø)
...ation.AspNet.TelemetryHttpModule/ActivityHelper.cs	85.71% <100.00%> (+0.46%)	⬆️
...nTelemetry.Instrumentation.AspNet/AspNetMetrics.cs	100.00% <100.00%> (ø)
...tion.AspNet/AspNetMetricsInstrumentationOptions.cs	100.00% <100.00%> (ø)
...ion.AspNet/Implementation/HttpInMetricsListener.cs	100.00% <100.00%> (ø)
...searchClient/ElasticsearchClientInstrumentation.cs	100.00% <100.00%> (ø)
.../ElasticsearchRequestPipelineDiagnosticListener.cs	82.35% <100.00%> (+0.14%)	⬆️
...ityFrameworkCore/EntityFrameworkInstrumentation.cs	80.00% <100.00%> (-20.00%)	⬇️
... and 47 more

... and 33 files with indirect coverage changes

[utpilla]

I don't see much value in extracting this into a variable. It's a single compile include item. I think extracting it into a variable is adding another layer of indirection to figure out what the project needs.

[CodeBlanch]

Seems like most project use it what if we made it included by default and only opted-out where a project didn't want to use it?

[utpilla]

My only issue with that would be that anytime someone contributes a new project to repo they would now have to be aware of this. We would probably have to update Contributing,md file for new contributors to look out for these "magically" added file(s).

I wouldn't want to surprise them 😄

[Kielek]

IMO we are in the worst place right now.
One part of the shared files is included by properties, the other part by direct Compile calls.

We should move all projects in one direction. When we have agreement, I am fine to extend contribution guide :) .

[utpilla]

IMO we are in the worst place right now.
One part of the shared files is included by properties, the other part by direct Compile calls.

I see value in extracting multiple Compile includes into a property. There are a bunch of projects that use a common set of Compile includes. But extracting a single compile include into a property doesn't make sense to me. If that's what we want to do then, where do we stop? Why don't we create a property for every single PackageReference and ProjectReference as well?

We should move all projects in one direction. When we have agreement, I am fine to extend contribution guide :) .

If we want to be consistent, I would be in favor of removing these properties altogether and have each project explicitly add the individual Compile Include they need.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.