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

Remove props file reference #25499

Merged
merged 1 commit into from
Mar 15, 2018
Merged

Remove props file reference #25499

merged 1 commit into from
Mar 15, 2018

Conversation

jaredpar
Copy link
Member

There is no props file in this package to Import. Having the Import here is breaking our ability to publish in official builds.

Ask Mode template not completed

Customer scenario

What does the customer do to get into this situation, and why do we think this
is common enough to address for this release. (Granted, sometimes this will be
obvious "Open project, VS crashes" but in general, I need to understand how
common a scenario is)

Bugs this fixes

(either VSO or GitHub links)

Workarounds, if any

Also, why we think they are insufficient for RC vs. RC2, RC3, or RTW

Risk

This is generally a measure our how central the affected code is to adjacent
scenarios and thus how likely your fix is to destabilize a broader area of code

Performance impact

(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")

Is this a regression from a previous update?

Root cause analysis

How did we miss it? What tests are we adding to guard against it in the future?

How was the bug found?

(E.g. customer reported it vs. ad hoc testing)

Test documentation updated?

If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.

There is no props file in this package to Import. Having the Import here is breaking our ability to publish in official builds.
@jaredpar jaredpar requested a review from a team as a code owner March 15, 2018 18:19
@jaredpar
Copy link
Member Author

CC @dotnet/roslyn-infrastructure for review

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

OK with this change, but we still don't understand what changed to break us. Do we have a root cause yet?

@333fred
Copy link
Member

333fred commented Mar 15, 2018

@tmat, can you take a look?

@jaredpar
Copy link
Member Author

@jasonmalinowski the root cause is this change

efcea52

It added a reference to the porps file which doesn't exist. I'm guessing this was just an oversight as you'd kind of expect such a props file to exist for this type of package. We don't have a way to test this except do an official build which is why this went unnoticed.

@tmat
Copy link
Member

tmat commented Mar 15, 2018

@jaredpar Could we instead make it conditional, in case the file is included in future version? I think I copied this from CoreFX repo and perhaps that was using older version of the package or something like that ...

@jaredpar
Copy link
Member Author

I'd rather not condition it. If they do add a props file it's likely to include the targets, as is standard for props files. Hence we'd move to double including the targets.

I chatted with @mmitche and he says they've never had this file.

@jaredpar jaredpar merged commit 52aedee into dotnet:dev15.7.x Mar 15, 2018
@jaredpar jaredpar deleted the fix-feed branch March 15, 2018 20:43
@nguerrera
Copy link
Contributor

If they do add a props file it's likely to include the targets, as is standard for props files.

Huh, it is not standard that props include targets.

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.

5 participants