Skip to content

[WIP] SDK node rename to Framework#3461

Closed
etbyrd wants to merge 1 commit intodotnet:masterfrom
etbyrd:sdk-name-change
Closed

[WIP] SDK node rename to Framework#3461
etbyrd wants to merge 1 commit intodotnet:masterfrom
etbyrd:sdk-name-change

Conversation

@etbyrd
Copy link
Copy Markdown
Contributor

@etbyrd etbyrd commented Apr 11, 2018

Customer scenario

N/A

Bugs this fixes:

Part of #2248

Workarounds, if any

N/A

Risk

Low, just renaming of files and the SDK node

Performance impact

None, just renaming

Is this a regression from a previous update?

No

Root cause analysis:

N/A

How was the bug found?

Requests from team members

@dnfclas
Copy link
Copy Markdown

dnfclas commented Apr 11, 2018

CLA assistant check
All CLA requirements met.

@etbyrd etbyrd changed the title SDK node rename to Framework [WIP] SDK node rename to Framework Apr 11, 2018
Copy link
Copy Markdown
Member

@Pilchie Pilchie left a comment

Choose a reason for hiding this comment

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

Looks okay, as long as we're all happy with making this change. Let's discuss a bit more in email to make sure everyone is happy with the change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should probably rename this as well.

@jmarolf jmarolf requested a review from a team April 12, 2018 00:40
@Nirmal4G
Copy link
Copy Markdown

Why rename the Sdk node, the previous issue comments show that we need both SDK node and the Framework Node, so Why not keep the SDK node and duplicate the SDK node into Framework node and put the SDK node under a feature flag or something so that we can enable them when the SDK resolvers implement them?!

It's just a waste to throw all of the work away!

@jmarolf
Copy link
Copy Markdown
Contributor

jmarolf commented Apr 14, 2018

@Nirmal4G this is a point-in-time thing. We have no intention of releasing Framework without SDK. It's just that Framework is a rather obvious name change compared to SDK which has more design considerations. No need to clutter this PR with SDK node design discussions.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 16, 2018

Codecov Report

Merging #3461 into master will increase coverage by <.01%.
The diff coverage is 75.86%.

@@            Coverage Diff             @@
##           master    #3461      +/-   ##
==========================================
+ Coverage   68.04%   68.05%   +<.01%     
==========================================
  Files         726      727       +1     
  Lines       35805    35810       +5     
  Branches     2095     2095              
==========================================
+ Hits        24364    24369       +5     
  Misses      11033    11033              
  Partials      408      408
Flag Coverage Δ
#debug 68.05% <75.86%> (ø) ⬆️
Impacted Files Coverage Δ
...o.ProjectSystem.Managed.VS/VSResources.Designer.cs 30.05% <0%> (ø) ⬆️
...Dependencies/Subscriptions/FrameworkRuleHandler.cs 0% <0%> (ø)
...ree/Dependencies/Models/SdkDependencyModelTests.cs 100% <100%> (ø) ⬆️
...t/SdkAndPackagesDependenciesSnapshotFilterTests.cs 100% <100%> (ø) ⬆️
...ilters/SdkAndPackagesDependenciesSnapshotFilter.cs 86.44% <100%> (ø) ⬆️
...ee/Dependencies/Models/FrameworkDependencyModel.cs 86.95% <100%> (ø)
.../LanguageServices/CSharpSyntaxFactsServiceTests.cs 100% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 243574a...4143d5e. Read the comment docs.

@davkean
Copy link
Copy Markdown
Member

davkean commented May 4, 2018

We're going to approach this a different way; we are adding a framework node.

@davkean davkean closed this May 4, 2018
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.

7 participants