-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Multiple warnings in source code #15015
Comments
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 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 🤖 🙂 |
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 :-) |
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 🤗 |
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) |
(Going to look into this as part of Umbraco UK Festival hackathon, if anyone else is then let me know!) |
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)! |
Data analysis: My build created 2299 errors, split over these 15 projects:
Data set: |
Removes warning from Umbraco.Web.UI as part of umbraco#15015
Removes warning from Umbraco.Web.UI as part of #15015
Nice - and now we can tick off the UI project too! |
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
|
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 |
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 |
Current status of 'contrib', 3636 warnings split over 19 projects:
As these are resolved, we can ensure warnings as errors are activated per project. |
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
Expected result / actual result
Minimal to no warnings ultimately
Tasks
The text was updated successfully, but these errors were encountered: