Skip to content

Clean up Core repos page table display#70

Merged
mashehu merged 4 commits into
mainfrom
claude/issue-60-20250823-0314
Dec 11, 2025
Merged

Clean up Core repos page table display#70
mashehu merged 4 commits into
mainfrom
claude/issue-60-20250823-0314

Conversation

@edmundmiller

Copy link
Copy Markdown
Contributor

Fixes #60

Claude implemented the Core repos page table improvements.

@edmundmiller edmundmiller requested a review from a team as a code owner August 23, 2025 03:20
@netlify

netlify Bot commented Aug 23, 2025

Copy link
Copy Markdown

Deploy Preview for nf-core-stats ready!

Name Link
🔨 Latest commit 6786155
🔍 Latest deploy log https://app.netlify.com/projects/nf-core-stats/deploys/693aa2d45be0620008a77de6
😎 Deploy Preview https://deploy-preview-70--nf-core-stats.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@claude

claude Bot commented Aug 23, 2025

Copy link
Copy Markdown
Contributor

Claude finished @edmundmiller's task —— View job


PR Review: Clean up Core repos page table display

I've reviewed this PR that fixes issue #60 for Core repos page table display improvements.

Review Tasks:

  • Read and analyze changed files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Review test coverage implications

Code Review Feedback

Code Quality & Best Practices

Strong improvements made:

  1. Explicit column selection: Replaced SELECT * with explicit column names in both SQL query (sources/nfcore_db/core_repos.sql:3-16) and Evidence.dev query (pages/code/core_repos.md:7-17), which is a best practice for maintainability and performance.

  2. Proper filtering: Added WHERE archived = false filter to exclude archived repositories from display (pages/code/core_repos.md:18).

  3. UI enhancements: Added proper DataTable closing tag and column width control (pages/code/core_repos.md:29-31).

Performance Considerations

Excellent performance improvements:

  1. Reduced data transfer: Explicit column selection eliminates unnecessary columns like _dlt_load_id and other metadata fields.
  2. Database-level sorting: Added ORDER BY stargazers_count DESC in SQL query (sources/nfcore_db/core_repos.sql:19) for efficient sorting.
  3. UI optimization: Fixed 300px width for description column prevents layout shifts and excessive table expansion.

Security Concerns

No security issues identified:

  • SQL queries use parameterized filtering with safe WHERE clauses
  • No dynamic SQL construction or user input concatenation
  • Proper use of Evidence.dev's built-in SQL templating system

⚠️ Minor Issues & Suggestions

  1. Column consistency: The SQL source query includes more columns (gh_pushed_at, topics, default_branch, category) than the Evidence.dev display query uses. Consider:

    • Either removing unused columns from the source query for cleaner data
    • Or utilizing these columns in the display (e.g., showing topics or default_branch)
  2. Data freshness: The Evidence.dev query doesn't include gh_pushed_at which could be valuable for showing repository activity.

Test Coverage

Testing recommendations:

  • Run npm run test to ensure Evidence.dev builds correctly with the new column specifications
  • Run npm run sources to refresh data sources and verify SQL query executes properly
  • Verify the UI renders correctly with the new maxWidth={300} constraint

📋 Summary

This is a well-executed cleanup that addresses the core table display issues:

Strengths:

  • Eliminates metadata columns from UI display
  • Improves performance through explicit column selection
  • Adds proper archived repository filtering
  • Implements sensible UI constraints for better layout

Impact: ✅ Positive - Clean, focused table display with better performance

Recommendation:Approve - The changes are solid improvements with no blocking issues.


@claude

claude Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

claude Bot and others added 2 commits December 11, 2025 11:06
- Exclude _dlt_load_id metadata column from display
- Filter out archived repositories
- Set fixed width (300px) for description column to prevent table expansion
- Add explicit column selection instead of SELECT *
- Improve sorting with ORDER BY stargazers_count DESC

Fixes #60

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Edmund Miller <edmundmiller@users.noreply.github.com>
@mashehu mashehu force-pushed the claude/issue-60-20250823-0314 branch from dca8e53 to d44b19a Compare December 11, 2025 10:26
@claude

claude Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-project-automation github-project-automation Bot moved this from Todo to In Progress in nf-core infrastructure projects Dec 11, 2025
@claude

claude Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@mashehu mashehu merged commit e6eec3b into main Dec 11, 2025
3 of 5 checks passed
@mashehu mashehu deleted the claude/issue-60-20250823-0314 branch December 11, 2025 10:55
@github-project-automation github-project-automation Bot moved this from In Progress to Done in nf-core infrastructure projects Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Clean up Core repos page table display

2 participants