Skip to content

4320 enable warnings as errors and fix any issues #4351

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
Closed
wants to merge 6 commits into from
Closed

4320 enable warnings as errors and fix any issues #4351

wants to merge 6 commits into from

Conversation

VineethReyya
Copy link
Contributor

Issue describing the changes in this PR

Fixing error by enabling warnings as errors using branch feature/foundation
resolves #4320

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

@VineethReyya VineethReyya requested review from a team as code owners April 7, 2025 14:21
@VineethReyya VineethReyya linked an issue Apr 7, 2025 that may be closed by this pull request
Fix warnings in the repository #4320
Open
3 tasks
@VineethReyya VineethReyya changed the base branch from main to feature/foundation April 7, 2025 14:22
@VineethReyya VineethReyya requested a review from aishwaryabh April 7, 2025 14:23
@VineethReyya
Copy link
Contributor Author

@aishwaryabh @liliankasem please review when you have a moment.
Thanks

aishwaryabh
aishwaryabh requested changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@aishwaryabh aishwaryabh left a comment

Choose a reason for hiding this comment

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

The all of the csproj's need to be updated to enable warnings as errors as well after making these changes

<TreatWarningsAsErrors>true</TreatWarningsAsErrors>

@VineethReyya
Copy link
Contributor Author

VineethReyya commented Apr 11, 2025 •
edited
Loading

The all of the csproj's need to be updated to enable warnings as errors as well after making these changes

<TreatWarningsAsErrors>true</TreatWarningsAsErrors>

@aishwaryabh updating <TreatWarningsAsErrors>true</TreatWarningsAsErrors> in Directory.Build.Common.props affecting all .csproj files. do we need externally add in all csproj's files?

Below is the screenshot after updating in Directory.Build.Common.props
image

@VineethReyya
Copy link
Contributor Author

The all of the csproj's need to be updated to enable warnings as errors as well after making these changes
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>

@aishwaryabh updating <TreatWarningsAsErrors>true</TreatWarningsAsErrors> in Directory.Build.Common.props affecting all .csproj files. do we need externally add in all csproj's files? also added changes. could you review once

Below is the screenshot after updating in Directory.Build.Common.props image

liliankasem
liliankasem reviewed Apr 14, 2025
View reviewed changes
src/Cli/func/Actions/HostActions/WebHost/Security/CliAuthenticationHandler.cs
public CliAuthenticationHandler(IOptionsMonitor<TOptions> options, ILoggerFactory logger, UrlEncoder encoder, ISystemClock clock)
: base(options, logger, encoder, clock)
public CliAuthenticationHandler(IOptionsMonitor<TOptions> options, ILoggerFactory logger, UrlEncoder encoder, TimeProvider timeProvider)
: base(options, logger, encoder)
Copy link
Member

Choose a reason for hiding this comment

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

do we not want to update the base to also taken in a time provider?

@VineethReyya
Copy link
Contributor Author

closing this because we created a new PR with refresh branch from main : #4365

@VineethReyya VineethReyya removed a link to an issue Apr 23, 2025
Fix warnings in the repository #4320
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liliankasem liliankasem liliankasem left review comments

@aishwaryabh aishwaryabh aishwaryabh requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@VineethReyya @liliankasem @aishwaryabh