Net6 #84

vnbaaij · 2021-10-05T22:08:50Z

Pull Request

📖 Description

The solution has been upgraded to .NET 6.

Hot reload has been enable,
Nullability has been enabled (and all compiler warnings fixed)
RTL/Theme switches have been moved to top of webcomponents page and work for all components now

🎫 Issues

This PR contains the work described in #78

👩‍💻 Reviewer Notes

📑 Test Plan

Go through all web componentspage items and validate correct working

✅ Checklist

General

I have added tests for my changes.
I have tested my changes.
I have updated the project documentation to reflect my changes.
I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

I have added a new component
I have modified an existing component

⏭ Next Steps

…to change inheritance to InputBase)

Make Theme switch work

…to change inheritance to InputBase)

Make Theme switch work

EisenbergEffect · 2021-10-11T20:46:03Z

.github/workflows/ci-daily.yml

@@ -2,9 +2,9 @@ name: CI - Validation for Blazor / FluentUI

 on:
  push:
-    branches: [ main ]
+    branches: [ net6 ]


This part doesn't seem correct. Shouldn't it be main still?
Same below.

Yes, it should be main once it is merged. Set it to net6 to be able to run the workflow in my fork

Gotcha. Sounds good. This looks nice! Thanks once again for putting in the work to make this happen.

EisenbergEffect · 2021-10-20T01:52:39Z

@vnbaaij Is this ready for final review? I'd love to merge and keep moving forward if you're ready.

vnbaaij · 2021-10-20T06:33:27Z

Yes, It is good to go!

EisenbergEffect

@vnbaaij Assuming it's all working in your branch, can you add another commit to switch the pipeline branch back to main?

@awentzel Do we need to make any updates to our pipelines or publish process in order to publish the .NET 6 package or can I just "click the magic button" once this is merged?

vnbaaij · 2021-10-21T10:03:46Z

latest commit reverts ci-daily back to main branch.

The Setup .NET action has been adjusted to use .NET 6 and pre release version:

- name: Setup .NET
      uses: actions/setup-dotnet@v1
      with:
        dotnet-version: 6.0.x
        include-prerelease: true

EisenbergEffect · 2021-10-21T12:34:38Z

@vnbaaij I'm seeing an error in our security scanning task as follows

error NETSDK1045: The current .NET SDK does not support targeting .NET 6.0. Either target .NET 5.0 or lower, or use a version of the .NET SDK that supports .NET 6.0.

Is this because .NET 6.0 is not yet released or because some other configuration option is incorrect or missing? I'm a little out of my comfort zone with some of this so I just wanted to double check before merging.

EisenbergEffect · 2021-10-21T12:35:50Z

@awentzel @nicholasrice Can you both do a quick review of the code. @vnbaaij has updated us to .NET 6.0 and taken advantage of some of the new features as appropriate. I'd like to get this merged, and then focus in on the current-value feature. With those two things done, we should be in a good position for the official .NET 6.0 release.

vnbaaij · 2021-10-21T13:02:41Z

Is this because .NET 6.0 is not yet released or because some other configuration option is incorrect or missing? I'm a little out of my comfort zone with some of this so I just wanted to double check before merging.
@EisenbergEffect yes, it is because .NET 6 has not yet been released. In the ci-daily.yml I explicitly had to include a setting for using prerelease version. Probably also need to do something like that for security scan.

EisenbergEffect · 2021-10-21T13:04:37Z

Is this because .NET 6.0 is not yet released or because some other configuration option is incorrect or missing? I'm a little out of my comfort zone with some of this so I just wanted to double check before merging.
@EisenbergEffect yes, it is because .NET 6 has not yet been released. In the ci-daily.yml I explicitly had to include a setting for using prerelease version. Probably also need to do something like that for security scan.

@awentzel Can you look into whether we can configure this for the security scan?

src/Microsoft.Fast.Components.FluentUI/FluentInputBase.cs

EisenbergEffect · 2021-10-25T21:05:13Z

@vnbaaij Thank you!
@awentzel I'd like to merge this but still would like to make sure we can move forward with the security scan issue. Time is getting short, so I don't want to hold much longer.

vnbaaij · 2021-10-25T21:36:34Z

@awentzel According to this page https://codeql.github.com/docs/codeql-overview/supported-languages-and-frameworks/ the security scanning does not work yet with .NET 6/C# 10/VS2022. I think you need to replace the autobuild step (https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-the-codeql-workflow-for-compiled-languages#adding-build-steps-for-a-compiled-language)

EisenbergEffect · 2021-10-26T23:09:26Z

@awentzel I know you are out today. Let's make this top priority for review when you are back in office. I'd like to get this merged ASAP but want to have you look at it first. See comments above.

awentzel · 2021-10-27T17:17:33Z

@awentzel Do we need to make any updates to our pipelines or publish process in order to publish the .NET 6 package or can I just "click the magic button" once this is merged?

I don't see any issues with the existing pipelines, so they should still work, with the exception of the CodeQL which has already been pointed out. I'll look into that next. Also, it would probably be best to do a publish immediately once this gets in and the main branch is updated to vet that part of the system so we don't run into any fire drills.

awentzel · 2021-10-27T17:39:02Z

https://codeql.github.com/docs/codeql-overview/supported-languages-and-frameworks/

@vnbaaij Thanks for digging into this. I tried to push a new commit to this branch and I don't have permissions so I will relay it here instead. I believe these are the "steps" we want to replace. Simply copy and paste over all existing steps in the codeql-analysis.yml with the following, essentially, adding Setup.Net step and manually build for csharp step.

steps:
    - name: Checkout repository
      uses: actions/checkout@v2

    - name: Setup .NET
      uses: actions/setup-dotnet@v1
      with:
        dotnet-version: 6.0.x
        include-prerelease: true
        
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v1
      with:
        languages: ${{ matrix.language }}

    - if: matrix.language == 'csharp' 
      name: Manually build CSharp on .NET 6
      run: |
        dotnet build

    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v1

It may require a little trial and error. If this commit doesn't immediately work, I'd recommend pushing into main anyhow, and then I can work through remaining code security configuration issues in a new PR.

awentzel · 2021-10-27T17:45:09Z

We probably will have to update the ADO pipeline as well for publishing to use pre-release, but once the code is in I can follow-up until that gets resolved. We use ADO to simplify code signing of internal libraries.

EisenbergEffect · 2021-10-27T17:47:47Z

@awentzel Just to confirm, should I go ahead and merge, and then you'll follow up with the fixes?

awentzel · 2021-10-27T17:51:26Z

@awentzel Just to confirm, should I go ahead and merge, and then you'll follow up with the fixes?

@EisenbergEffect yes, let's do that.

EisenbergEffect · 2021-10-27T17:52:34Z

Done.

vnbaaij added 25 commits September 28, 2021 15:41

Upgrade projects to .NET 6

8ed173d

Enable nullable for sample project

3f5766f

Enable Nullable for component project (excluded that changes related …

e9c0f74

…to change inheritance to InputBase)

Enable Nullable for FluentInputBase derivates

79cdfb1

Update component project to only optimie on Release build

da49498

Update component project to only optimize on Release build

0fe610b

Merge branch 'net6' of https://github.com/vnbaaij/fast-blazor into net6

598bd46

Revert SetParametersAsync logic

59b7768

Move RTL/Theme switches to webcomponents page

7032a8d

Enable Hot Reload

2f50d53

Make Theme switch work

Upgrade projects to .NET 6

8bd654a

Enable nullable for sample project

b29ab95

Enable Nullable for component project (excluded that changes related …

9be1dc4

…to change inheritance to InputBase)

Enable Nullable for FluentInputBase derivates

ed029ba

Revert SetParametersAsync logic

f01b491

Move RTL/Theme switches to webcomponents page

212e431

Enable Hot Reload

fd775b7

Make Theme switch work

Merge

03c1d15

Fix compiler warnings/messages

49372e0

Change CI validation to use .NET 6 (prerelease)

5b545c0

Add link to Fluent UI sample site

2b674d9

Update CI Validation workflow to net 6 branch

f116db4

Nullable work

0ed4cb0

Nullable work

02dc918

chore: prepare 0.5.0 release

cd1acb3

EisenbergEffect requested review from nicholasrice, EisenbergEffect and awentzel October 11, 2021 20:42

EisenbergEffect reviewed Oct 11, 2021

View reviewed changes

Update to rc2

9ed89e6

Merge branch 'main' into net6

a471ec7

EisenbergEffect self-requested a review October 20, 2021 18:49

EisenbergEffect approved these changes Oct 20, 2021

View reviewed changes

Reverted ci-daily back to main branch

a13e67e

Merge branch 'net6' of https://github.com/vnbaaij/fast-blazor into net6

342f090

vnbaaij added 2 commits October 23, 2021 16:51

Fix DataGridTest

479daa4

Fix FluentNumberField binding

b8f21f2

nicholasrice reviewed Oct 25, 2021

View reviewed changes

src/Microsoft.Fast.Components.FluentUI/FluentInputBase.cs Outdated Show resolved Hide resolved

nicholasrice approved these changes Oct 25, 2021

View reviewed changes

Remove commented code

cd05c7f

awentzel approved these changes Oct 27, 2021

View reviewed changes

EisenbergEffect merged commit fc00f86 into microsoft:main Oct 27, 2021

vnbaaij deleted the net6 branch November 1, 2021 19:55

Net6 #84

Net6 #84

Uh oh!

Conversation

vnbaaij commented Oct 5, 2021

Pull Request

📖 Description

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

Component-specific

⏭ Next Steps

Uh oh!

EisenbergEffect Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

vnbaaij Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

EisenbergEffect Oct 11, 2021

Choose a reason for hiding this comment

Uh oh!

EisenbergEffect commented Oct 20, 2021

Uh oh!

vnbaaij commented Oct 20, 2021

Uh oh!

EisenbergEffect left a comment

Choose a reason for hiding this comment

Uh oh!

vnbaaij commented Oct 21, 2021

Uh oh!

EisenbergEffect commented Oct 21, 2021

Uh oh!

EisenbergEffect commented Oct 21, 2021

Uh oh!

vnbaaij commented Oct 21, 2021

Uh oh!

EisenbergEffect commented Oct 21, 2021

Uh oh!

Uh oh!

EisenbergEffect commented Oct 25, 2021

Uh oh!

vnbaaij commented Oct 25, 2021

Uh oh!

EisenbergEffect commented Oct 26, 2021

Uh oh!

awentzel commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awentzel commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awentzel commented Oct 27, 2021

Uh oh!

EisenbergEffect commented Oct 27, 2021

Uh oh!

awentzel commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EisenbergEffect commented Oct 27, 2021

Uh oh!

Uh oh!

awentzel commented Oct 27, 2021 •

edited

Loading

awentzel commented Oct 27, 2021 •

edited

Loading

awentzel commented Oct 27, 2021 •

edited

Loading