Multiple warnings in source code

[emmagarland]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

12.3.0+ (but applies to any version)
v14 (latest)

Bug summary

There are thousands of warnings within the Umbraco source code. I was hoping to try and start to reduce these as use this issue as a base to work from (unless there already is one?).

My idea is to go through each project and check that there are no warnings, and if there are, remove them.

This is until perhaps we can set warnings as errors to keep it clean for future? Bigger step/decision! I have started an initial PR for this at #15019.

Specifics

I recommend ticking off each project if there are no warnings, or if all warnings have since been removed.
To maintain this, we would also set warnings as errors (something for HQ to consider)

Steps to reproduce

1. Open IDE of choice
2. Build
3. Observe warnings

Expected result / actual result

Minimal to no warnings ultimately

Tasks

Beta Give feedback

1. Remove warnings from Templates/Umbraco.Templates project
2. Remove warnings from Tests/Umbraco.TestData project
3. Remove warnings from Tests/Umbraco.Tests.AcceptanceTest project
4. Remove warnings from Tests/Umbraco.Tests.Benchmarks project
5. Remove warnings from Tests/Umbraco.Tests.Common project
6. Remove warnings from Tests/Umbraco.Tests.Integration project
7. Remove warnings from Tests/Umbraco.Tests.UnitTests project
8. Remove warnings from Tools/Umbraco.JsonSchema project
9. Remove warnings from Umbraco.Cms project
10. Remove warnings from Umbraco.Cms.Api.Common project
11. Remove warnings from Umbraco.Cms.Api.Delivery project
12. Remove warnings from Umbraco.Cms.Imaging.ImageSharp project
13. Remove warnings from Umbraco.Cms.Imaging.ImageSharp2 project
14. Remove warnings from Umbraco.Cms.Persistence.EFCore project
15. Remove warnings from Umbraco.Cms.Persistence.EFCore.Sqlite project
16. Remove warnings from Umbraco.Cms.Persistence.EFCore.SqlServer project
17. Remove warnings from Umbraco.Cms.Persistence.Sqlite project
18. Remove warnings from Umbraco.Cms.Persistence.SqlServer project
19. Remove warnings from Umbraco.Cms.StaticAssets project
20. Remove warnings from Umbraco.Cms.Targets project
21. Remove warnings from Umbraco.Core project
22. Remove warnings from Umbraco.Examine.Lucene project
23. Remove warnings from Umbraco.Infrastructure project
24. Remove warnings from Umbraco.PublishedCache.NuCache project
25. Remove warnings from Umbraco.Web.BackOffice project
26. Remove warnings from Umbraco.Web.Common project
27. Remove warnings from Umbraco.Web.UI project
28. Remove warnings from Umbraco.Web.UI.Client project
29. Remove warnings from Umbraco.Web.Website project

[github-actions]

Hi there @emmagarland!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[github-actions]

Hi @emmagarland,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post.

Thanks muchly, from your friendly Umbraco GitHub bot :-)

[Migaroez]

Hey Emma!

As a new HQ'er this is something that popped on my radar as well and I am very pleased to see some community initiative on this.

I will talk this trough with the team on what our vision for this is, feel free to continue making smaller PR's and ill get back to you asap 🤗

[Zeegaan]

This would be great! I did some work on this a while ago, just trying to relieve some of this, by reformatting our code files: #12438 (this is just for the core project)

[emmagarland]

Thanks @Migaroez and @Zeegaan! Glad that you like the idea, looking forward to hearing more.

Nice to see you did a bunch of updates already @Zeegaan 👍

Emma

[jacksorjacksor]

(Going to look into this as part of Umbraco UK Festival hackathon, if anyone else is then let me know!)

[emmagarland]

Sounds good @jacksorjacksor !

I have a PR open to set warnings as errors on projects without warnings, but @Migaroez is going to let me know HQ stance on that.

In the meantime, removing warnings is fair game for all (there are lots)!

[jacksorjacksor]

Data analysis:

My build created 2299 errors, split over these 15 projects:

Project	Count
Umbraco.Infrastructure	700
Umbraco.Tests.Integration	512
Umbraco.Web.BackOffice	316
Umbraco.Tests.UnitTests	302
Umbraco.Core	269
Umbraco.PublishedCache.NuCache	102
Umbraco.Cms.Persistence.SqlServer	31
Umbraco.Web.Common	24
Umbraco.Tests.Benchmarks	18
Umbraco.Tests.Common	7
Umbraco.Web.Website	6
Umbraco.Cms.Persistence.Sqlite	5
Umbraco.Cms.Persistence.EFCore	3
Umbraco.Examine.Lucene	3
Umbraco.Web.UI	1
Grand Total	2299

Data set:
BuildWarnings.csv

[emmagarland]

Data analysis:

My build created 2299 errors, split over these 15 projects:

|Project|Count|

|---|---|

|Umbraco.Infrastructure|700|

|Umbraco.Tests.Integration|512|

|Umbraco.Web.BackOffice|316|

|Umbraco.Tests.UnitTests|302|

|Umbraco.Core|269|

|Umbraco.PublishedCache.NuCache|102|

|Umbraco.Cms.Persistence.SqlServer|31|

|Umbraco.Web.Common|24|

|Umbraco.Tests.Benchmarks|18|

|Umbraco.Tests.Common|7|

|Umbraco.Web.Website|6|

|Umbraco.Cms.Persistence.Sqlite|5|

|Umbraco.Cms.Persistence.EFCore|3|

|Umbraco.Examine.Lucene|3|

|Umbraco.Web.UI|1|

|Grand Total|2299|

Data set:

BuildWarnings.csv

Nice - and now we can tick off the UI project too!

[Migaroez]

Hey y'all

We had a chat about this internally and I don't think it comes as a surprise that we would love all the warnings to go away as well.

Below are a couple of pointers/ideas

• Keep the PR's coming but keep them small(ish) to avoid double work and support faster merging, don't forget to reference this ticket.
• We can keep this ticket open as a tracking ticket if @emmagarland doesn't mind maintaining it.
• We will further internally discus when the right time would be to turn on warnings as errors.
• We are making our Internal review process a bit stricter to hopefully catch any new warnings from popping up.

[emmagarland]

All sounds brilliant, thanks @Migaroez for following this all up!

Agreed re small PRs.

Happy to keep this open as a tracking ticket, and I'll keep on top of it.

Looking forward to the outcome of this!

Emma

[emmagarland]

Hey @Migaroez, thanks for merging the first phase of this task!

I'll reopen this issue to keep it as a tracking issue if that's cool.

Cheers,

Emma

[emmagarland]

Data analysis:

My build created 2299 errors, split over these 15 projects:

Project Count
Umbraco.Infrastructure 700
Umbraco.Tests.Integration 512
Umbraco.Web.BackOffice 316
Umbraco.Tests.UnitTests 302
Umbraco.Core 269
Umbraco.PublishedCache.NuCache 102
Umbraco.Cms.Persistence.SqlServer 31
Umbraco.Web.Common 24
Umbraco.Tests.Benchmarks 18
Umbraco.Tests.Common 7
Umbraco.Web.Website 6
Umbraco.Cms.Persistence.Sqlite 5
Umbraco.Cms.Persistence.EFCore 3
Umbraco.Examine.Lucene 3
Umbraco.Web.UI 1
Grand Total 2299
Data set: BuildWarnings.csv

Current status of 'contrib', 3636 warnings split over 19 projects:

1. Umbraco.Cms.Api.Delivery: 16
2. Umbraco.Cms.Api.Management: 92
3. Umbraco.Cms.Persistence.EFCore: 9
4. Umbraco.Cms.Persistence.SqlServer: 31
5. Umbraco.Cms.Persistence.Sqlite: 89
6. Umbraco.Cms.Targets: 525
7. Umbraco.Core: 525
8. Umbraco.Examine.Lucene: 10
9. Umbraco.Infrastructure: 697
10. Umbraco.JsonSchema: 34
11. Umbraco.PublishedCache.NuCache: 81
12. Umbraco.TestData: 11
13. Umbraco.Tests.Benchmarks: 15
14. Umbraco.Tests.Common: 5
15. Umbraco.Tests.Integration: 1269
16. Umbraco.Tests.UnitTests: 319
17. Umbraco.Web.Common: 40
18. Umbraco.Web.UI: 1 (cannot currently find this warning)
19. Umbraco.Web.Website: 13

As these are resolved, we can ensure warnings as errors are activated per project.