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

Export rodeo ingestor doh #44

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Export rodeo ingestor doh #44

merged 1 commit into from
Aug 26, 2024

Conversation

chrismaddern
Copy link
Contributor

@chrismaddern chrismaddern commented Aug 26, 2024

PR Type

enhancement


Description

  • Added RodeoIngestor to the list of available mint ingestors.
  • Updated the ALL_MINT_INGESTORS map to include the new RodeoIngestor.

Changes walkthrough 📝

Relevant files
Enhancement
index.ts
Add RodeoIngestor to Mint Ingestors                                           

src/ingestors/index.ts

  • Added import statement for RodeoIngestor.
  • Included RodeoIngestor in the ALL_MINT_INGESTORS map.
  • +2/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @chrismaddern chrismaddern requested a review from agans August 26, 2024 13:47
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Update type definition to include all specific ingestors

    Consider updating the MintIngestionMap type to include the new rodeo ingestor for
    better type checking and documentation.

    src/ingestors/index.ts [10-12]

     export type MintIngestionMap = {
    -  [key: string]: MintIngestor;
    +  'prohibition-daily': MintIngestor;
    +  fxhash: MintIngestor;
    +  highlight: MintIngestor;
    +  transient: MintIngestor;
    +  foundation: MintIngestor;
    +  'zora-internal': MintIngestor;
    +  rodeo: MintIngestor;
     };
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Updating the MintIngestionMap type to explicitly include all specific ingestors enhances type checking and documentation, making the code more robust and easier to understand. This is a significant improvement in terms of code clarity and maintainability.

    8
    Best practice
    Add type annotation to ensure consistency and type safety

    Consider adding a type annotation to the rodeo property in the ALL_MINT_INGESTORS
    object to ensure type safety and consistency with other ingestors.

    src/ingestors/index.ts [21]

    -rodeo: new RodeoIngestor(),
    +rodeo: new RodeoIngestor() as MintIngestor,
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a type annotation to the rodeo property improves type safety and consistency with other ingestors, which is a good practice in TypeScript. However, it is not crucial since the MintIngestionMap already enforces the type.

    7

    @chrismaddern chrismaddern merged commit 0b86f99 into main Aug 26, 2024
    1 check passed
    @chrismaddern chrismaddern deleted the chris/export-rodeo branch August 26, 2024 13:48
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants