Skip to content

[new-hs-indexer #17] Index instance declarations and class defaults - #551

Closed
simonmar wants to merge 1 commit into
facebookincubator:mainfrom
simonmar:hs-indexer-inst-binds
Closed

[new-hs-indexer #17] Index instance declarations and class defaults#551
simonmar wants to merge 1 commit into
facebookincubator:mainfrom
simonmar:hs-indexer-inst-binds

Conversation

@simonmar

Copy link
Copy Markdown
Collaborator

We can now, for example, find all the places where a particular method
is defined by an instance.

@netlify

netlify Bot commented Jun 27, 2025

Copy link
Copy Markdown

Deploy Preview for fb-oss-glean ready!

Name Link
🔨 Latest commit 3d90706
🔍 Latest deploy log https://app.netlify.com/projects/fb-oss-glean/deploys/686fa6a93f3fbd00082ddb69
😎 Deploy Preview https://deploy-preview-551--fb-oss-glean.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 27, 2025
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@phlalx has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@simonmar
simonmar force-pushed the hs-indexer-inst-binds branch from 0c646c8 to 1490d88 Compare June 29, 2025 14:22
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@phlalx has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@simonmar
simonmar force-pushed the hs-indexer-inst-binds branch 3 times, most recently from 7ad8145 to ad423ba Compare July 2, 2025 12:09
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@phlalx has imported this pull request. If you are a Meta employee, you can view this in D77440727.


predicate InstanceDecl:
{
name: Name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can only remove defaultable fields from predicate. Was this on purpose?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes, it was deliberate. I'll need to rename it or bump the version again...

@simonmar
simonmar marked this pull request as draft July 8, 2025 14:24
@simonmar
simonmar force-pushed the hs-indexer-inst-binds branch from ad423ba to 817d5d9 Compare July 10, 2025 09:54
@simonmar
simonmar marked this pull request as ready for review July 10, 2025 11:15
We can now, for example, find all the places where a particular method
is defined by an instance.
@simonmar
simonmar force-pushed the hs-indexer-inst-binds branch from 817d5d9 to 3d90706 Compare July 10, 2025 11:40
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@pepeiborra has imported this pull request. If you are a Meta employee, you can view this in D77440727.

class_: ClassDecl |
method: MethDecl |
instance: InstanceDecl |
instance: InstDecl |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This type change breaks Thrift compat.

Given that this is a rename, can it be safely ignored?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You can always ignore Thrift compat issues, but I guess in this case it will also break schema validation... let me check

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's weird, schema validation succeeds, but I think it should fail. Could be a bug.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ah no, I know why: it's because the change only affects derived predicates. It's always safe to change derived predicates because there's no data translation needed.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants