Skip to content

Conversation

Evangelink
Copy link
Contributor

Description

Enable the nullable feature on GitVersion.Core project.

Related Issue

Fixes #2764

@arturcic arturcic requested review from arturcic and asbjornu July 19, 2021 16:41
@Evangelink
Copy link
Contributor Author

I will rebase this PR once the readonly and the analyzers got merged.

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

Great stuff!

@Evangelink Evangelink requested a review from asbjornu July 22, 2021 07:55
Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

This is great! I love that it's now visible how many opportunities for NullReferenceException we have in the codebase. 😅 That's something I would very much like to remedy, though. I'm not saying all ? can or should be removed, but I do think there's way too many in the current codebase.

If you see any places where null guards would alleviate the need for null values, please sprinkle them throughout the codebase as you see fit. As long as the tests are green; the more null guards and less ? we have, the better.

That's also something we can do in a followup PR. If you'd like this merged now, that's fine by me.

@asbjornu asbjornu merged commit 73345d9 into GitTools:main Jul 22, 2021
@mergify
Copy link
Contributor

mergify bot commented Jul 22, 2021

Thank you @Evangelink for your contribution!

@arturcic
Copy link
Member

@Evangelink thank you for the effort. I know what this kind of refactoring is so many thanks. Hope we can improve the codebase even more so that we can prepare it for version 6.0 (and target net 6.0 as it's LTS and drop support for net48 and netcore3.1)

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.

[Improvement] Enable nullable on GitVersion.Core
3 participants