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.

Sign up for GitHub

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

Jump to bottom

Eliminate All Compiler Warnings In Solution #2149

Open
mark-monteiro opened this issue Dec 13, 2019 · 12 comments · Fixed by #6667 · May be fixed by #11473
Open

Eliminate All Compiler Warnings In Solution #2149

mark-monteiro opened this issue Dec 13, 2019 · 12 comments · Fixed by #6667 · May be fixed by #11473
Labels
confirmed This issue has been reviewed and confirmed good first issue Good for newcomers
Projects
Cleanup

Comments

@mark-monteiro
Copy link
Member

mark-monteiro commented Dec 13, 2019 •
edited by barronpm

This is a general ticket to track progress on eliminating all warnings in the solution source code, including enabling missing analyzers and converting warnings to errors in release builds. @Bond-009, as I understand, you have already been working on this for a while. I thought I would start helping out with this task and we (and anyone else) can coordinate progress here.

The table below summarizes the progress on this task at the current HEAD commit on master:

Project Warnings FxCop Analyzer StyleCop Analyzer Serilog Analyzer Multithreading Analyzer Warnings As Errors
DvdLib
Emby.Dlna
Emby.Drawing
Emby.Naming
Emby.Notifications
Emby.Photos
Emby.Server.Implementations
  • (644 Warnings)
Jellyfin.Api
Jellyfin.Data
Jellyfin.Drawing.Skia
Jellyfin.Server
Jellyfin.Server.Implementations
MediaBrowser.Common
MediaBrowser.Controller
  • (1053 Warnings)
MediaBrowser.LocalMetadata
MediaBrowser.MediaEncoding
MediaBrowser.Model
  • (2132 Warnings)
MediaBrowser.Providers
  • (538 Warnings)
MediaBrowser.XbmcMetadata
RSSDP
Jellyfin.Api.Tests
Jellyfin.Common.Tests
Jellyfin.Controller.Tests
Jellyfin.MediaEncoding.Tests
Jellyfin.Model.Tests
Jellyfin.Naming.Tests
Jellyfin.Server.Implementations.Tests
MediaBrowser.Api.Tests
Total 4367 Warnings
This was referenced Dec 13, 2019
Closed
Closed
Closed
Merged
@anthonylavado anthonylavado mentioned this issue Dec 20, 2019
Merged
@KuraiAndras KuraiAndras mentioned this issue Dec 24, 2019
Closed
This was referenced Jan 31, 2020
Merged
Merged
Merged
@crobibero crobibero mentioned this issue Mar 25, 2020
Merged
@Bond-009 Bond-009 added this to To do in Cleanup Apr 5, 2020
@barronpm barronpm mentioned this issue Apr 11, 2020
Closed
@dtparr dtparr mentioned this issue Apr 13, 2020
Closed
This was referenced Apr 14, 2020
Merged
Closed
Closed
@mark-monteiro mark-monteiro added the good first issue Good for newcomers label Apr 19, 2020
@kekonn kekonn mentioned this issue Apr 20, 2020
Closed
@dkanada dkanada pinned this issue Apr 27, 2020
@crobibero crobibero mentioned this issue May 14, 2020
Merged
@telans telans mentioned this issue Jun 13, 2020
Merged
@crobibero crobibero mentioned this issue Jun 14, 2020
Merged
This was referenced Jun 14, 2020
Merged
Merged
Merged
Merged
@jvoisin
Copy link
Contributor

jvoisin commented Jun 26, 2020

Would it be possible to add LGTM.com to the list? It found a lot of issues.

This was referenced Aug 4, 2020
Merged
Merged
@daullmer daullmer mentioned this issue Aug 8, 2020
Closed
This was referenced Aug 10, 2021
Merged
Merged
Merged
@crobibero crobibero mentioned this issue Aug 28, 2021
Merged
@Bond-009 Bond-009 mentioned this issue Sep 9, 2021
Merged
This was referenced Oct 2, 2021
Merged
Merged
@KonH KonH mentioned this issue Oct 3, 2021
Merged
Bond-009 pushed a commit that referenced this issue Oct 3, 2021
@KonH
Fix warning: The nullable warning suppression expression is redundant (…
7c282ec 
…#2149)
Bond-009 pushed a commit that referenced this issue Oct 3, 2021
@KonH
Fix warning: Using directive is not required by the code and can be s…
b6bf43a 
…afely removed (#2149)
Bond-009 pushed a commit that referenced this issue Oct 3, 2021
@KonH
Fix warning: Qualifier is redundant (#2149)
e3fccd5
Bond-009 pushed a commit that referenced this issue Oct 3, 2021
@KonH
Fix warning: Redundant control flow jump statement (#2149)
d45fcdd
Bond-009 pushed a commit that referenced this issue Oct 3, 2021
@KonH
Fix warning: Type cast is redundant (#2149)
6cbfdea
@Bond-009 Bond-009 mentioned this issue Oct 6, 2021
Merged
Cleanup automation moved this from To do to Done Oct 8, 2021
@Bond-009 Bond-009 reopened this Sep 9, 2022
Cleanup automation moved this from Done to In progress Sep 9, 2022
@Christian-Oleson
Copy link

I'm willing to help out with this if you are looking for help

@JJS JJS mentioned this issue Oct 6, 2022
Merged
@jbatt33
Copy link

jbatt33 commented Mar 30, 2023

I just built Jellyfin and I'm not seeing any warnings. Is this bug fixed or am I looking at the wrong repo. Maybe I need to build harder.

@Bond-009
Copy link
Member

@jbatt33 are you building in debug mode?

@jbatt33
Copy link

jbatt33 commented Mar 30, 2023

Yes, I did both debug and release builds of the jellyfin under VS 2022 and did not get any warnings or errors, but get 3 messages:
image

@Bond-009
Copy link
Member

I think it's an issue with dotnet, if you remove <CodeAnalysisTreatWarningsAsErrors>false</CodeAnalysisTreatWarningsAsErrors> from the projects that have it you'll get build errors

@jbatt33
Copy link

jbatt33 commented Mar 31, 2023

Thanks, I deleted that line from the project file and it's working as expected. I'm going to see if I can tackle any of these.

@Daaiid Daaiid mentioned this issue Apr 13, 2023
Merged
@MattCost MattCost mentioned this issue May 13, 2023
Closed
@LoremFooBar
Copy link

I'd like to try to clear some of these warnings. I already started with the simpler ones. Some warnings require refactoring that is easy to do, but hard to determine whether they are breaking changes. I'd like to try to tackle these as well later on.

@gsedubun
Copy link

gsedubun commented May 1, 2024

I'd like to try to clear some of these.

@gsedubun gsedubun linked a pull request May 1, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed This issue has been reviewed and confirmed good first issue Good for newcomers
Projects
Cleanup
  
In progress
Jellyfin - Triage Board
Status: In Progress
Milestone
No milestone
9 participants
@jvoisin @LoremFooBar @mark-monteiro @gsedubun @dcparga @Bond-009 @dkanada @jbatt33 @Christian-Oleson