Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore(core): CNX-7435 - Resolved even more core warnings #3109

Merged
merged 9 commits into from
Dec 19, 2023

Conversation

JR-Morgan
Copy link
Member

@JR-Morgan JR-Morgan commented Dec 13, 2023

Main changes with this PR:

  • Supressed warnings on obsolete functions (since they are effectively "resolved")
  • Cleared up the last the "easy to fix" warnings in core. The rest need some level of planning (although to varying degrees)
    Just 274 left!
  • I've made nullability syntax default in core since we had more exceptions (classes with #nullable enabled than we had implicitly disabled), this should also encourage us to design new classes with clear nullability in mind

@JR-Morgan JR-Morgan added core issues related to the .net sdk. tech debt labels Dec 13, 2023
@JR-Morgan JR-Morgan added this to the 2.18 milestone Dec 13, 2023
@JR-Morgan JR-Morgan self-assigned this Dec 13, 2023
@JR-Morgan JR-Morgan marked this pull request as draft December 13, 2023 00:32
@JR-Morgan
Copy link
Member Author

Going to resolve the integration tests failing in the morning

@JR-Morgan JR-Morgan marked this pull request as ready for review December 13, 2023 15:21
@JR-Morgan JR-Morgan requested review from gjedlicska and removed request for gjedlicska December 13, 2023 15:21
Copy link
Member

@AlanRynne AlanRynne left a comment

Choose a reason for hiding this comment

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

I think I've got mostly questions... 🤕 🙌🏼

Core/Core/Kits/Attributes.cs Outdated Show resolved Hide resolved
Core/Core/Kits/ISpeckleConverter.cs Show resolved Hide resolved
Core/Core/Kits/KitDeclaration.cs Show resolved Hide resolved
Core/Core/Kits/Units.cs Show resolved Hide resolved
Core/Core/Kits/Units.cs Show resolved Hide resolved
Core/Core/Logging/Setup.cs Show resolved Hide resolved
Core/Core/Models/Utilities.cs Show resolved Hide resolved
Core/Core/Serialisation/BaseObjectDeserializerV2.cs Outdated Show resolved Hide resolved
Core/Core/Transports/SQLite.cs Show resolved Hide resolved
@BovineOx
Copy link
Contributor

Added my 2 cents

Copy link
Member

@AlanRynne AlanRynne left a comment

Choose a reason for hiding this comment

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

Many, many conversations later, this is ready to go 🚀

@AlanRynne AlanRynne merged commit 7b9592e into dev Dec 19, 2023
30 checks passed
@AlanRynne AlanRynne deleted the jrm/core/warningsagain branch December 19, 2023 17:08
JR-Morgan added a commit that referenced this pull request Dec 19, 2023
* More warnings resolved

* More warnings

* chore(core): Explicit disable nullability syntax for select files

* chore(core): Nullability now default

* more exception doc

* Fix tests

* The last warnings

* removed closure throw and tracked issue for further investigation

* Requested changes
JR-Morgan added a commit that referenced this pull request Dec 19, 2023
* More warnings resolved

* More warnings

* chore(core): Explicit disable nullability syntax for select files

* chore(core): Nullability now default

* more exception doc

* Fix tests

* The last warnings

* removed closure throw and tracked issue for further investigation

* Requested changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core issues related to the .net sdk. tech debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants