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: Standardize comment format to JSDoc-style syntax #1868

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

cpcramer
Copy link
Contributor

@cpcramer cpcramer commented Jan 23, 2025

What changed? Why?

chore: Standardize comment format to JSDoc-style syntax

We're implementing a consistent approach to documentation comments across our codebase. This will improve readability and make our documentation more maintainable.

New Comment Standards

  1. Multi-line comments (/** */) for:

    • Function definitions
    • Class definitions
    • Type/Interface definitions
    • Complex exports
  2. Single-line comments (/** */) for:

    • Properties within types/interfaces
    • Simple constants
    • Basic exports
  3. Regular comments (//) for:

    • Inside function implementations
    • Implementation details
    • Temporary notes
    • Line-by-line explanations

Example:

/**
 * Note: exported as public Type
 */
export type isEthereumOptions = {
  /** Chain ID for the network */
  chainId: number;
  /** If the chainId check is only allowed on mainnet */
  isMainnetOnly?: boolean;
};

/**
 * DismissableLayer handles dismissal using outside clicks and escape key events
 */
export function DismissableLayer({}) {
      // This prevents the popover from closing when clicking inside it
      if (layerRef.current?.contains(event.target as Node)) {
        return;
      }

      // Handling for the trigger button (e.g., settings toggle)
      // Without this, clicking the trigger would cause both:
      // 1. The button's onClick to fire (toggling isOpen)
      // 2. This dismissal logic to fire (forcing close)
      // This would create a race condition where the popover rapidly closes and reopens
      const isTriggerClick = (event.target as HTMLElement).closest(
        '[aria-label="Toggle swap settings"]',
      );
}

Result:
Screenshot 2025-01-23 at 12 54 20 PM

Notes to reviewers

How has it been tested?

Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
onchainkit-coverage ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 4:52pm
onchainkit-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 4:52pm
onchainkit-routes ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 4:52pm

@cpcramer cpcramer force-pushed the paul/update-comments branch from 024c51d to cc16d07 Compare January 23, 2025 20:11
@cpcramer cpcramer force-pushed the paul/update-comments branch from 75f2f55 to 8ba1b4f Compare January 23, 2025 20:23
brendan-defi
brendan-defi previously approved these changes Jan 24, 2025
toAmount: string; // The amount of the destination token
toAmountUSD: string; // The USD value of the destination token
warning?: QuoteWarning; // The warning associated with the quote
/** The reference amount for the quote */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: i don't know what a reference amount is, and this comment doesn't help me understand

brendan-defi
brendan-defi previously approved these changes Jan 24, 2025
@cpcramer cpcramer requested a review from brendan-defi January 24, 2025 17:01
@cpcramer cpcramer merged commit d23ad4c into main Jan 24, 2025
16 checks passed
@cpcramer cpcramer deleted the paul/update-comments branch January 24, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants