Skip to content

Print warning when AddJsonFile is called with "local.settings.json"#2777

Open
BogdanYarotsky wants to merge 27 commits intoAzure:mainfrom
BogdanYarotsky:local-settings-json-analyzer
Open

Print warning when AddJsonFile is called with "local.settings.json"#2777
BogdanYarotsky wants to merge 27 commits intoAzure:mainfrom
BogdanYarotsky:local-settings-json-analyzer

Conversation

@BogdanYarotsky
Copy link
Copy Markdown

@BogdanYarotsky BogdanYarotsky commented Oct 13, 2024

Issue describing the changes in this PR

resolves #613

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • 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)

Additional information.

Hi @fabiocav, this is my first pull request in this repository. I've added a bunch of unit tests and documentation for the new analyzer. Please let me know what you think about this PR and what else should be added to it.

Many Thanks!
Bogdan

@BogdanYarotsky BogdanYarotsky changed the title Local settings json analyzer Print warning when AddJsonFile is called with "local.settings.json" Oct 13, 2024
@BogdanYarotsky
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Comment thread sdk/Sdk.Analyzers/DiagnosticDescriptors.cs Outdated
Comment thread docs/analyzer-rules/AZFW0016.md Outdated
@BogdanYarotsky
Copy link
Copy Markdown
Author

Hi, sure, I will update the PR soon.

@BogdanYarotsky
Copy link
Copy Markdown
Author

Hello @liliankasem, I've updated the ID of the warning. Please let me know if any other changes are needed. Thank you for the review!

@Frulfump
Copy link
Copy Markdown

Frulfump commented Jun 17, 2025

This seems useful could we get an update here? @liliankasem (@kshyju @mattchenderson)

@liliankasem
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@jviau
Copy link
Copy Markdown
Contributor

jviau commented Feb 11, 2026

/azp run dotnet-worker.public

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jviau
Copy link
Copy Markdown
Contributor

jviau commented Feb 11, 2026

@BogdanYarotsky apologies for the long delay on this. It seems like there is some build issue now. It is pointing to a file not in this PR at all, so I imagine it might be a merge issue or something stale. Can you rebase off of main?

@BogdanYarotsky
Copy link
Copy Markdown
Author

@jviau thank you for coming back on this one. I definitely can, my only concern is that's a recurring pattern with this PR. It becomes stale, updates are required from my side, after me implementing it, the PR goes stale again after some time. What can we do to avoid this situation in the future and merge it after next changes if everything is alright?

One of the main challenges with analyzers specifically is that they rely on manual increment of id, which leads to git conflicts if few analyzers are contributed in parallel. If such feedback is valued, I could open a new issue about it.

I would be happy to bring this PR to mergeable state, please let me know how to proceed.

Thanks!

@jviau
Copy link
Copy Markdown
Contributor

jviau commented Feb 19, 2026

@BogdanYarotsky the back-and-forth staleness of the PR is on us, we did not dedicate the time to drive this PR to completion. We are dedicating more time to resolving PRs going forward. Apologies again.

The analyzer ID conflict is understandable. Not sure the best way to do that because it needs to be part of docs. Probably a story for another time, if doable at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Print warning when AddJsonFile is called with "local.settings.json"

5 participants