Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

perforce: Add fallback logic for changelist mapper#62871

Merged
eseliger merged 1 commit into
mainfrom
es/05-23-perforceaddfallbacklogicforchangelistmapper
May 29, 2024
Merged

perforce: Add fallback logic for changelist mapper#62871
eseliger merged 1 commit into
mainfrom
es/05-23-perforceaddfallbacklogicforchangelistmapper

Conversation

@eseliger

Copy link
Copy Markdown
Member

When the changelist mapper has not yet indexed a changelist ID for fast lookup, we try to find it via git log instead to bridge the time between a git fetch and the new commit SHAs being indexed better.

Test plan:

Verified locally that with an empty DB changelist IDs still show up.

@cla-bot cla-bot Bot added the cla-signed label May 22, 2024

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @eseliger and the rest of your teammates on Graphite Graphite

@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels May 22, 2024
@eseliger eseliger force-pushed the es/05-23-perforceaddfallbacklogicforchangelistmapper branch from 94e10fc to d258aea Compare May 22, 2024 22:23
@eseliger eseliger marked this pull request as ready for review May 23, 2024 16:26
@eseliger eseliger requested review from a team and peterguy May 23, 2024 16:26
@eseliger eseliger force-pushed the es/05-23-perforceaddfallbacklogicforchangelistmapper branch from d258aea to 73bfcc2 Compare May 23, 2024 16:30

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.

small preference: would prefer having dedicate variable name for this (would help with setting breakpoints if I want to)

Suggested change
MessageQuery: fmt.Sprintf("change = %d]", cid),
messageQuery := fmt.Sprintf("change = %d]", cid)
MessageQuery: messageQuery,

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.

I see that we're swallowing the error here. Can we possibly log it/ add it to the trace?

@eseliger eseliger force-pushed the es/05-23-perforceaddfallbacklogicforchangelistmapper branch from 73bfcc2 to 45ce7f7 Compare May 29, 2024 09:27
When the changelist mapper has not yet indexed a changelist ID for fast lookup, we try to find it via git log instead to bridge the time between a git fetch and the new commit SHAs being indexed better.

Test plan:

Verified locally that with an empty DB changelist IDs still show up.
@eseliger eseliger force-pushed the es/05-23-perforceaddfallbacklogicforchangelistmapper branch from 45ce7f7 to 819282c Compare May 29, 2024 09:33
@eseliger eseliger enabled auto-merge (squash) May 29, 2024 09:41
@eseliger eseliger merged commit cafb6fb into main May 29, 2024
@eseliger eseliger deleted the es/05-23-perforceaddfallbacklogicforchangelistmapper branch May 29, 2024 09:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants