Skip to content
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

chore: remove redundant SingleShardInfo type #2086

Open
weboko opened this issue Jul 24, 2024 · 2 comments
Open

chore: remove redundant SingleShardInfo type #2086

weboko opened this issue Jul 24, 2024 · 2 comments
Labels
debt Technical debt marker enhancement New feature or request

Comments

@weboko
Copy link
Collaborator

weboko commented Jul 24, 2024

This is a change request

Problem

Both ShardInfo and SingleShardInfo at the same time make experience confusing and prone to errors as they are not compatible with each other.

Proposed Solutions

Use only ShardInfo.

@weboko weboko added enhancement New feature or request debt Technical debt marker labels Jul 24, 2024
@weboko weboko added this to Waku Jul 24, 2024
@weboko weboko moved this to Triage in Waku Jul 24, 2024
@danisharora099 danisharora099 moved this from To Do to Triage in Waku Nov 29, 2024
@danisharora099
Copy link
Collaborator

SingleShardInfo represents a combination of one shard and one clusterId, while ShardInfo represents multiple shards and one clusterId.

Taking an example of where SingleShardInfo is used, for example creating a decoder:
Providing multiple shards would map to multiple pubsub topics, which is against the design of creating one decoder: one decoder maps to one pubsub topic

If we deprecate SingleShardInfo and use ShardInfo, this would break the design of creating a decoder

@weboko
Copy link
Collaborator Author

weboko commented Dec 3, 2024

This is a valid concern and I agree. We can keep SingleShardInfo as an alias.

Let's re-iterate:

SingleShardInfo -

export interface SingleShardInfo {

ShardInfo -

export type ShardInfo = {

Because they are not compatible - we have to use mapping function, it's easy to get confused and we got bugs in the past.

type SingleShardInfo = {
  clusterId: number;
  shards: [number];
};

wdyt, @danisharora099 ?

@weboko weboko moved this from Triage to To Do in Waku Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Technical debt marker enhancement New feature or request
Projects
Status: To Do
Development

No branches or pull requests

2 participants