Skip to content

Commit

Permalink
agent: update comments, turn them into TODO's
Browse files Browse the repository at this point in the history
  • Loading branch information
tilacog committed Aug 11, 2023
1 parent 0e258aa commit d9f2d7c
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions packages/indexer-agent/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ export class Agent {
deploymentIDSet(targetDeployments),
)
if (isReconciliationNeeded) {
// QUESTION: should we return early in here case reconciliation is not needed?
// TODO: Return early in here case reconciliation is not needed
this.logger.debug('Reconcile deployments', {
syncing: activeDeployments.map(id => id.display),
target: targetDeployments.map(id => id.display),
Expand All @@ -1029,8 +1029,9 @@ export class Agent {
!deploymentInList(eligibleAllocationDeployments, deployment),
)

// QUESTION: Same as before: should we return early in here case reconciliation is
// not needed?
// TODO: Same as before: Should we return early in here case reconciliation is not needed? Also,
// this conditional seems the be doing the same work as the previous one, we should consolidate
// them.
if (deploy.length + remove.length !== 0) {
this.logger.info('Deployment changes', {
deploy: deploy.map(id => id.display),
Expand Down Expand Up @@ -1132,7 +1133,7 @@ export class Agent {
epoch,
})

// QUESTION: Can we replace `filter` for `find` here? Is there such a case when we
// TODO: Can we replace `filter` for `find` here? Is there such a case when we
// would have multiple allocations for the same subgraph?
const activeDeploymentAllocations = activeAllocations.filter(
allocation =>
Expand Down Expand Up @@ -1184,8 +1185,6 @@ export class Agent {
}
}

// QUESTION: the `activeAllocations` parameter is used only for logging. Should we
// remove it from this function?
async reconcileActions(
networkDeploymentAllocationDecisions: NetworkMapped<AllocationDecision[]>,
epoch: NetworkMapped<number>,
Expand Down

0 comments on commit d9f2d7c

Please sign in to comment.