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

Remove Prohibition Daily #66

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Remove Prohibition Daily #66

merged 1 commit into from
Dec 16, 2024

Conversation

chrismaddern
Copy link
Contributor

@chrismaddern chrismaddern commented Dec 16, 2024

User description

Mint Ingestor:

Functionality Supported

  • Ingesting from URL: Yes / No
  • Ingesting from Contract address: Yes / No
  • Supported Networks: Base / Ethereum / Arbitrum / Zora / Polygon / Others..

Before you submit

  • Ensure your generated MintTemplate works 😄
  • Ensure that your code is restricted to a single folder in src/ingestors
  • Ensure that all required assets are included (e.g. ABIs)
  • Ensure ABIs are trimmed to include only methods (1) used in the ingestor or (2) required to mint
  • Ensure that all exported methods are prefixed with the name of your ingestor e.g. myMintingPlatformGetContractDetails
  • Ensure that a test exists for generating a MintTemplate that will always succeed
  • Ensure that your code accesses no external resources except those passed in the resources object

PR Type

Other


Description

  • Removed the Prohibition Daily ingestor implementation and all associated files
  • Removed Prohibition Daily from the list of supported platforms in ALL_MINT_INGESTORS
  • Updated documentation to reflect the removal of Prohibition Daily support
  • Cleaned up formatting in README.md

Changes walkthrough 📝

Relevant files
Configuration changes
index.ts
Remove Prohibition Daily ingestor registration                     

src/ingestors/index.ts

  • Removed ProhibitionDailyIngestor import and reference from
    ALL_MINT_INGESTORS map
  • +1/-3     
    Enhancement
    *
    Remove Prohibition Daily ingestor implementation                 

    src/ingestors/prohibition-daily/*

  • Removed entire prohibition-daily folder containing ingestor
    implementation
  • Deleted ABI definitions, contract interaction logic, and metadata
    handling
  • Removed associated test files
  • Documentation
    README.md
    Update documentation to remove Prohibition Daily references

    README.md

  • Removed Prohibition Daily from supported platforms table
  • Updated example PR reference from Prohibition Daily to VV Mint
    Ingestor
  • Minor formatting improvements
  • +47/-54 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @chrismaddern chrismaddern merged commit ab3a4bd into main Dec 16, 2024
    1 check passed
    @chrismaddern chrismaddern deleted the chris/remove-prohibition branch December 16, 2024 15:46
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Style
    The trailing comma is missing after adding 'coinbase-wallet' to ALL_MINT_INGESTORS map. Consider adding it for consistency with the rest of the codebase.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add missing import statement for a newly referenced class to prevent runtime errors

    The 'coinbase-wallet' ingestor is added but its import statement is missing. Add the
    import to maintain code consistency and prevent runtime errors.

    src/ingestors/index.ts [1-5]

     import { MintIngestor } from '../lib/types/mint-ingestor';
     import { ManifoldIngestor } from './manifold';
     import { RaribleIngestor } from './rarible';
     import { FxHashIngestor } from './fxhash';
     import { TransientIngestor } from './transient-base';
    +import { CoinbaseWalletIngestor } from './coinbase-wallet';
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion identifies a critical missing import that would cause runtime errors, as CoinbaseWalletIngestor is used in the ALL_MINT_INGESTORS map but not imported. This is a significant issue that needs to be fixed for the code to work properly.

    9

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

    Successfully merging this pull request may close these issues.

    1 participant