Skip to content

[homekit] Add nullability to (generated and manual) bindings - #15111

Merged
tj-devel709 merged 6 commits into
dotnet:mainfrom
tj-devel709:Nullable-HomeKit
Jun 6, 2022
Merged

tj-devel709 merged 6 commits into
dotnet:mainfrom
tj-devel709:Nullable-HomeKit

Conversation

@tj-devel709

Copy link
Copy Markdown
Member

This PR aims to bring nullability changes to HomeKit.
Following the steps here:

  1. I am looking for any !missing-null-allowed! in this frameworks ignore files and applying them
  2. I am adding nullable enable to all manual files that are not "API_SOURCES" in src/frameworks.sources and making the required nullability changes
  3. Changing any == null or != null to is null and is not null

@tj-devel709 tj-devel709 added the not-notes-worthy Ignore for release notes label May 24, 2022
@tj-devel709 tj-devel709 added this to the Future milestone May 24, 2022
var constant = significantEvent.GetConstant ();
if (constant is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (significantEvent));
return CreatePredicateForEvaluatingTriggerOccurringBeforeSignificantEvent (constant, offset);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This method and CreatePredicateForEvaluatingTriggerOccurringAfterSignificantEvent below both eventually throw an ArgumentNullException so we could ! the significantEvent.GetConstant () and just pass it on but I thought this may be better. Opinions welcome!

Comment thread src/HomeKit/HMHome.cs
return GetServices (arr.ToArray ());
}

class ServiceTypeList<T> : List<T> {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This saves a ton of null checks above from lines 17 - 80, but this may be hiding the null check. Opinions?

@vs-mobiletools-engineering-service2

This comment has been minimized.

@mandel-macaque

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@tj-devel709

Copy link
Copy Markdown
Member Author

Hmm the legacy xtro tests are failing here, but they pass locally?

@tj-devel709

Copy link
Copy Markdown
Member Author

Did not mean to close issue, instead meant to include this image of xtro running this branch locally
image

@tj-devel709 tj-devel709 reopened this May 25, 2022
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1036.Monterey
Hash: 37099b0dde974ad351e577d3e4ed5237b5df95cd

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [PR Build] Tests on macOS Mac Catalina (10.15) failed ❌

Failed tests are:

  • monotouch-test

Pipeline on Agent
Hash: 37099b0dde974ad351e577d3e4ed5237b5df95cd

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

📋 [PR Build] API Diff 📋

API diff (for current PR)

ℹ️ API Diff (from PR only) (please review changes)

API diff: vsdrops gist

Xamarin
.NET
Xamarin vs .NET
iOS vs Mac Catalyst (.NET)

API diff (vs stable)

✅ API Diff from stable

API diff: vsdrops gist

Xamarin
.NET
Xamarin vs .NET
iOS vs Mac Catalyst (.NET)

Generator diff

ℹ️ Generator Diff (please review changes)

Pipeline on Agent XAMBOT-1108.Monterey'
Hash: 37099b0dde974ad351e577d3e4ed5237b5df95cd

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: 37099b0dde974ad351e577d3e4ed5237b5df95cd

@vs-mobiletools-engineering-service2

Copy link
Copy Markdown
Collaborator

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

1 tests failed, 147 tests passed.

Failed tests

  • link all/watchOS 32-bits - simulator/Debug: Crashed

Pipeline on Agent XAMBOT-1169.Monterey'
Merge 37099b0 into 9822cbc

@tj-devel709

Copy link
Copy Markdown
Member Author

@tj-devel709
tj-devel709 merged commit 9bf39cd into dotnet:main Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

not-notes-worthy Ignore for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants