Skip to content

Enable nullable annotations in ServiceControl.Persistence - #5774

Open
rbev wants to merge 3 commits into
masterfrom
rhys/enable-nullable
Open

Enable nullable annotations in ServiceControl.Persistence#5774
rbev wants to merge 3 commits into
masterfrom
rhys/enable-nullable

Conversation

@rbev

@rbev rbev commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

This PR enables nullable annotations in ServiceControl.Persistence

The change is strives to avoid any functional code changes to above the persistence seam where possible by codifying the existing usage into the annotations based upon object creation or consumption patterns.

@rbev rbev self-assigned this Aug 14, 2026

@johnsimons johnsimons left a comment

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.

Just a few questions along the way

Comment thread src/ServiceControl.Persistence.EFCore/Implementation/EventLogDataStore.cs Outdated
Body = storedBody,
BodyStoredExternally = storeExternally,
ExceptionInfo = failure.ExceptionInfo
ExceptionInfo = failure.ExceptionInfo ?? ""

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 use string.Empty?

Comment thread src/ServiceControl.Persistence.EFCore/Implementation/FailedMessageViewMapper.cs Outdated
Comment thread src/ServiceControl.Persistence.EFCore/Implementation/FailedMessageViewMapper.cs Outdated
Comment thread src/ServiceControl.Persistence.RavenDB/EndpointDetailsParser.cs
Comment thread src/ServiceControl.Persistence.Tests/EFCore/PersistenceTestsContext.cs Outdated
{
public string Id { get; set; }
public object DispatchContext;
public string? Id { get; set; }

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.

that is weird? the id can be null!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These null ids are because they are DB generated ids

}

public string Id { get; set; }
public string? Id { get; set; }

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.

weird again?

@rbev
rbev force-pushed the rhys/enable-nullable branch from e803e18 to 856cca5 Compare August 14, 2026 07:25
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.

2 participants