Skip to content

Filter for functions marked as unknown - Issue #5 #7

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pintorem
Copy link

I'm sharing my progress on issue #5 by opening this PR draft. Any advice or comments would be greatly appreciated as I become more familiar with the XRefer codebase!

So far, I have saved the settings and applied the UI update. This component seems to be working well.
I've attempted to apply the filter to functions that are marked as unknown, but I'd like to know if this is the right way to do it.

Copy link

google-cla bot commented Mar 30, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@pintorem pintorem marked this pull request as draft March 30, 2025 07:33
@m-umairx
Copy link
Collaborator

m-umairx commented Apr 5, 2025

This is great @pintorem! So can you make a small change, we basically want the UI based check box to enable or disable ALL identified library functions regardless of whether they're unknown or not. However, when the setting is enabled and library functions are being included in the analysis, then we want to go ahead and exclude functions that are unknown as they don't provide any meaningful benefit. So essentially:

  1. the UI setting should enable inclusion/exclusion of ALL library functions
  2. when library functions are enabled, unknown functions should be excluded by default.

Let me know if there are further questions. Apologies on the delayed response, I'll be prompt if you have any further questions. Otherwise the PR looks good.

@pintorem pintorem marked this pull request as ready for review April 8, 2025 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants