Skip to content

Improve PropertyChanged.Fody Code Quality #3495

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

Merged
merged 2 commits into from
Apr 30, 2025
Merged

Improve PropertyChanged.Fody Code Quality #3495

merged 2 commits into from
Apr 30, 2025

Conversation

Jack251970
Copy link
Contributor

Set private assets all for PropertyChanged.Fody

Fix build warning `MSBUILD : warning FodyPackageReference: Fody: The package reference for PropertyChanged.Fody does not contain PrivateAssets='All'

Change function name and remove unused function

Fix build warning

PluginViewModel.cs(159,9,159,10): warning : Fody/PropertyChanged: Type Flow.Launcher.ViewModel.PluginViewModel contains a method OnActionKeywordsChanged which will not be called as ActionKeywords is not found. You can suppress this warning with [SuppressPropertyChangedWarnings].
PluginViewModel.cs(164,9,164,10): warning : Fody/PropertyChanged: Type Flow.Launcher.ViewModel.PluginViewModel contains a method OnSearchDelayTimeChanged which will not be called as SearchDelayTime is not found. You can suppress this warning with [SuppressPropertyChangedWarnings].

@Jack251970 Jack251970 self-assigned this Apr 30, 2025
@prlabeler prlabeler bot added the bug Something isn't working label Apr 30, 2025
Copy link

gitstream-cm bot commented Apr 30, 2025

🚨 gitStream Monthly Automation Limit Reached 🚨

Your organization has exceeded the number of pull requests allowed for automation with gitStream.
Monthly PRs automated: 267/250

To continue automating your PR workflows and unlock additional features, please contact LinearB.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves code quality regarding PropertyChanged.Fody by addressing build warnings.

  • Renames OnActionKeywordsChanged to OnActionKeywordsTextChanged in PluginViewModel
  • Removes the unused OnSearchDelayTimeChanged method to eliminate Fody warnings
  • Updates ActionKeywords.xaml.cs to call the renamed method

Reviewed Changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.

File Description
Flow.Launcher/ViewModel/PluginViewModel.cs Renamed method and removed unused method to resolve Fody warnings
Flow.Launcher/ActionKeywords.xaml.cs Updated call to reflect the method renaming
Files not reviewed (3)
  • Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj: Language not supported
  • Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj: Language not supported
  • Flow.Launcher/Flow.Launcher.csproj: Language not supported

@Jack251970 Jack251970 requested a review from jjw24 April 30, 2025 03:01
@Jack251970 Jack251970 removed the bug Something isn't working label Apr 30, 2025
Copy link

gitstream-cm bot commented Apr 30, 2025

🥷 Code experts: no user but you matched threshold 10

Jack251970 has most 👩‍💻 activity in the files.
Jack251970 has most 🧠 knowledge in the files.

See details

Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj

Activity based on git-commit:

Jack251970
APR 1 additions & 0 deletions
MAR 0 additions & 1 deletions
FEB
JAN 1 additions & 0 deletions
DEC 8 additions & 0 deletions
NOV

Knowledge based on git-blame:
Jack251970: 13%

Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj

Activity based on git-commit:

Jack251970
APR
MAR
FEB
JAN
DEC 9 additions & 1 deletions
NOV

Knowledge based on git-blame:
Jack251970: 11%

Flow.Launcher/ActionKeywords.xaml.cs

Activity based on git-commit:

Jack251970
APR 11 additions & 16 deletions
MAR
FEB 102 additions & 61 deletions
JAN 1 additions & 1 deletions
DEC
NOV 2 additions & 1 deletions

Knowledge based on git-blame:
Jack251970: 61%

Flow.Launcher/Flow.Launcher.csproj

Activity based on git-commit:

Jack251970
APR
MAR 1 additions & 5 deletions
FEB 11 additions & 10 deletions
JAN 4 additions & 2 deletions
DEC 4 additions & 0 deletions
NOV

Knowledge based on git-blame:
Jack251970: 4%

Flow.Launcher/ViewModel/PluginViewModel.cs

Activity based on git-commit:

Jack251970
APR 37 additions & 48 deletions
MAR 94 additions & 57 deletions
FEB 31 additions & 37 deletions
JAN
DEC
NOV

Knowledge based on git-blame:
Jack251970: 41%

To learn more about /:\ gitStream - Visit our Docs

Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors and Warnings Count
❌ forbidden-pattern 22
⚠️ non-alpha-in-dictionary 19

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Copy link

gitstream-cm bot commented Apr 30, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

Copy link
Contributor

coderabbitai bot commented Apr 30, 2025

📝 Walkthrough

Walkthrough

This update modifies project files to mark the PropertyChanged.Fody package as a private asset, preventing it from being included as a transitive dependency for downstream projects. Additionally, a method in PluginViewModel is renamed from OnActionKeywordsChanged to OnActionKeywordsTextChanged, and the method OnSearchDelayTimeChanged is removed. Corresponding changes are made in ActionKeywords.xaml.cs to invoke the renamed method. No changes are made to the public interface except for the method rename and removal.

Changes

File(s) Change Summary
Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj,
Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj,
Flow.Launcher/Flow.Launcher.csproj
Added <PrivateAssets>all</PrivateAssets> to the PropertyChanged.Fody package reference to prevent transitive dependency exposure.
Flow.Launcher/ViewModel/PluginViewModel.cs Renamed OnActionKeywordsChanged to OnActionKeywordsTextChanged. Removed OnSearchDelayTimeChanged.
Flow.Launcher/ActionKeywords.xaml.cs Updated method call from OnActionKeywordsChanged() to OnActionKeywordsTextChanged().

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ActionKeywordsWindow
    participant PluginViewModel

    User->>ActionKeywordsWindow: Replace action keyword
    ActionKeywordsWindow->>PluginViewModel: OnActionKeywordsTextChanged()
    PluginViewModel->>PluginViewModel: Notify property change for ActionKeywordsText
Loading

Suggested reviewers

  • jjw24

Poem

In the warren of code, a bunny did hop,
Tidying up packages, making the leaks stop.
A method renamed, another said goodbye,
Dependencies tucked in, so nothing will fly.
With a wiggle and twitch, the rabbit’s delight—
Clean code and neat assets, all tucked in tight!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b36562a and 6b9a710.

📒 Files selected for processing (5)
  • Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj (1 hunks)
  • Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj (1 hunks)
  • Flow.Launcher/ActionKeywords.xaml.cs (1 hunks)
  • Flow.Launcher/Flow.Launcher.csproj (1 hunks)
  • Flow.Launcher/ViewModel/PluginViewModel.cs (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Flow.Launcher/ActionKeywords.xaml.cs (1)
Flow.Launcher/ViewModel/PluginViewModel.cs (1)
  • OnActionKeywordsTextChanged (158-161)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: gitStream workflow automation
  • GitHub Check: gitStream.cm
  • GitHub Check: build
🔇 Additional comments (6)
Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj (1)

69-71: Good addition of PrivateAssets tag for PropertyChanged.Fody

Adding <PrivateAssets>all</PrivateAssets> to the PropertyChanged.Fody package reference is correct. This prevents the package from being included as a transitive dependency for projects that reference this one, addressing the MSBUILD warning mentioned in the PR objectives.

Flow.Launcher/Flow.Launcher.csproj (1)

101-103: Good addition of PrivateAssets tag for PropertyChanged.Fody

Adding <PrivateAssets>all</PrivateAssets> to the PropertyChanged.Fody package reference is correct. This prevents the package from being included as a transitive dependency for projects that reference this one, addressing the MSBUILD warning mentioned in the PR objectives.

Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj (1)

79-81: Good addition of PrivateAssets tag for PropertyChanged.Fody

Adding <PrivateAssets>all</PrivateAssets> to the PropertyChanged.Fody package reference is correct. This prevents the package from being included as a transitive dependency for projects that reference this one, addressing the MSBUILD warning mentioned in the PR objectives.

Flow.Launcher/ActionKeywords.xaml.cs (1)

83-83: Correctly updated method name to match property name

The method call has been updated from OnActionKeywordsChanged() to OnActionKeywordsTextChanged(), which matches the actual property name being changed (ActionKeywordsText). This fixes the Fody warning about methods not being called because the corresponding properties were not found.

Flow.Launcher/ViewModel/PluginViewModel.cs (2)

158-161: Good renaming to match property notification

The method has been appropriately renamed from OnActionKeywordsChanged to OnActionKeywordsTextChanged to better reflect that it's notifying changes for the ActionKeywordsText property (defined on line 150). This change addresses the PropertyChanged.Fody warnings about methods not being called because of property name mismatches.


150-156:

✅ Verification successful

Verify that the removed OnSearchDelayTimeChanged method isn't needed

I notice that OnSearchDelayTimeChanged method has been removed (it was previously in the file according to the PR description). This appears to be intentional to address Fody warnings, but please verify that no other code was calling this method. The property SearchDelayTimeText on line 151-153 now has no explicit change notification method.


🏁 Script executed:

#!/bin/bash
# Search for any references to OnSearchDelayTimeChanged to ensure it's not called anywhere
rg "OnSearchDelayTimeChanged" --type cs

Length of output: 39


No references to OnSearchDelayTimeChanged found
I ran a repository-wide search for OnSearchDelayTimeChanged and confirmed there are no calls or dependencies on this method. It’s safe to keep it removed.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Jack251970 Jack251970 merged commit f45e6ac into dev Apr 30, 2025
13 checks passed
@Jack251970 Jack251970 deleted the fody_property branch April 30, 2025 04:00
@Jack251970 Jack251970 added this to the 1.20.0 milestone Apr 30, 2025
@jjw24 jjw24 added the bug Something isn't working label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Code Quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants