-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Split SEO generation options for static pages and posts #143
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
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSplit SEO generation into explicit static, posts, and per-post flows; add generator factory (newSEOGenerator/runSEOGeneration) returning a *seo.Generator; add CLI/menu wiring and slug capture; refactor Generator to expose GenerateStaticPages, GeneratePosts, and GeneratePost; update tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Menu
participant CLI as main.go
participant Gen as seo.Generator
User->>Menu: Choose SEO option
Menu->>CLI: dispatch case (4/5/6)
CLI->>CLI: runSEOGeneration / newSEOGenerator()
CLI->>Gen: (case 4) GenerateStaticPages()
CLI->>Gen: (case 5) GeneratePosts()
CLI->>Gen: (case 6) GeneratePost(slug)
Gen-->>CLI: success / error
CLI-->>User: report result
sequenceDiagram
autonumber
participant OldFlow as Old: Generate()
participant NewFlow as New: Split flows
OldFlow->>OldFlow: Single Generate() (static + posts)
note right of OldFlow: Mixed responsibilities
NewFlow->>NewFlow: GenerateStaticPages()
NewFlow->>NewFlow: GeneratePosts()
NewFlow->>NewFlow: GeneratePost(slug) [optional]
note right of NewFlow: Explicit, per-target generation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces enhanced flexibility to the SEO generation utility by separating the process for static pages and blog posts. Previously, a single operation handled both; now, users can choose to generate SEO for each content type independently via new CLI menu options. This change improves usability and allows for more targeted updates, potentially saving time and resources when only one type of content needs SEO regeneration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively splits the SEO generation process into two distinct CLI options for static pages and blog posts. The changes are well-structured across the CLI entrypoint, the SEO generator logic, and the corresponding tests. I have one suggestion to improve maintainability by reducing some code duplication in metal/cli/main.go. Overall, this is a solid improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
metal/cli/main.go (1)
160-190: Consider consolidating the helper functions.The
generateStaticSEO()andgeneratePostsSEO()functions follow identical patterns: create generator, then call a specific method. While this is functional, you could reduce duplication with a helper that accepts the method as a parameter.Optional refactor:
func generateStaticSEO() error { return generateSEOWith(func(gen *seo.Generator) error { return gen.GenerateStaticPages() }) } func generatePostsSEO() error { return generateSEOWith(func(gen *seo.Generator) error { return gen.GeneratePosts() }) } func generateSEOWith(fn func(*seo.Generator) error) error { gen, err := newSEOGenerator() if err != nil { return err } return fn(gen) }However, given the simplicity and clarity of the current approach, this refactor is entirely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
metal/cli/main.go(2 hunks)metal/cli/panel/menu.go(1 hunks)metal/cli/seo/generator.go(2 hunks)metal/cli/seo/generator_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
metal/cli/seo/generator.go (1)
pkg/cli/message.go (1)
Magentaln(33-35)
metal/cli/main.go (2)
metal/cli/seo/generator.go (2)
Generator(30-38)NewGenerator(40-94)pkg/portal/validator.go (1)
GetDefaultValidator(23-33)
🔇 Additional comments (6)
metal/cli/panel/menu.go (1)
92-94: LGTM! Menu options updated correctly.The menu text accurately reflects the split SEO generation functionality, with clear labeling for static pages (option 4) and blog posts (option 5). The new timestamp option (option 6) is appropriately placed.
metal/cli/seo/generator_test.go (1)
155-161: LGTM! Tests properly updated for two-phase generation.The test correctly invokes both
GenerateStaticPages()andGeneratePosts()in sequence, matching the new API design. Error handling and messages are appropriate for each phase.metal/cli/seo/generator.go (3)
96-102: LGTM! Clean delegation pattern.The
Generate()method now acts as a convenience wrapper that orchestrates both static pages and blog posts generation. This maintains backward compatibility while exposing the new granular methods.
104-125: LGTM! Well-structured static page generation.The new
GenerateStaticPages()method cleanly encapsulates the static page pipeline with appropriate logging. The step-based approach with deferred completion logging is clear and maintainable.
295-298: LGTM! Consistent logging added.The logging statements provide clear visibility into the blog posts generation lifecycle, matching the pattern used in
GenerateStaticPages().metal/cli/main.go (1)
70-82: LGTM! Case statements correctly updated.The switch cases properly map to the new menu options, with case 4 handling static SEO generation, case 5 handling posts SEO generation, and case 6 handling timestamp printing. Error handling is consistent across all cases.
53693f8 to
d38c75f
Compare
Summary
Summary by CodeRabbit
New Features
Refactor
UX
Tests
Content