Skip to content

Conversation

@asafashirov
Copy link
Contributor

@asafashirov asafashirov commented Nov 21, 2025

Adds new "Pulumi Guides" menu item under "For Engineers" section in the main navigation with link to /guides/ post-launch.

@claude
Copy link
Contributor

claude bot commented Nov 21, 2025

Documentation Review

I've reviewed the changes in this PR. Here are my findings:

Issues Found

1. Broken Link Target (Critical)

File: layouts/partials/header.html:246 and layouts/partials/header.html:462

The navigation links point to /guides/, but this page does not exist in the content directory. I confirmed that:

  • There is no content/guides/_index.md file
  • There is no content/guides.md file
  • Only nested guides exist under content/docs/iac/guides/, content/docs/esc/guides/, etc.

Impact: Users clicking this menu item will encounter a 404 error, which is a poor user experience and breaks navigation.

Recommendation: Before merging, ensure that:

  1. A content/guides/_index.md file is created with appropriate content, OR
  2. The link target is changed to an existing guides page (e.g., /docs/iac/guides/), OR
  3. Coordinate with the team to ensure the /guides/ page will exist when this PR is deployed

Style Issues

2. Inconsistent Formatting

File: layouts/partials/header.html:247-249

The subtitle text should be reviewed for consistency with other menu items. The subtitle reads: "Practical infrastructure patterns for cloud providers"

While grammatically correct, consider whether "cloud providers" is the right scope or if "cloud platforms" or "infrastructure challenges" might be more appropriate, depending on the actual content planned for the guides section.

HTML Structure Review

The HTML changes are properly structured and consistent with existing menu items:

  • ✅ Uses the same <li> and <div class="list-title"> structure
  • ✅ Includes Font Awesome icon (fa-book-open)
  • ✅ Follows the pattern of other menu items with title and subtitle
  • ✅ Added to both desktop navigation (line 244-252) and mobile navigation (line 462-464)
  • ✅ Positioned logically after "Engineers love Pulumi"

Summary

The implementation is technically sound and follows the existing navigation patterns correctly. However, this PR should not be merged until the /guides/ landing page exists to prevent broken links.


@asafashirov Please verify that the /guides/ page will be available when this is deployed, or update the link target accordingly. Feel free to mention me (@claude) if you'd like additional review after addressing the broken link issue.

@pulumi-bot
Copy link
Collaborator

@asafashirov
Copy link
Contributor Author

Pending Pulumi Guides launch.

@claude
Copy link
Contributor

claude bot commented Dec 2, 2025

Documentation Review

I've reviewed the changes to add the Pulumi Guides menu item to the main navigation. The implementation looks good overall with just a few minor observations:

Issues Found

1. Text formatting consistency (layouts/partials/header.html:249)

The subtitle uses "cloud providers" which is technically accurate, but consider if "cloud platforms" might be more consistent with Pulumi's typical terminology for AWS, Azure, GCP, etc.

                                                <div class="list-sub-title">Practical infrastructure patterns for cloud platforms</div>

Observations

Positive aspects:

  • Both desktop dropdown and mobile menu implementations are included
  • Consistent icon usage (fa-book-open) across both implementations
  • Link structure follows the existing pattern with trailing slashes
  • HTML structure and indentation match the surrounding code

Note: The link points to /guides/ which should exist or be ready for launch. Ensure that the target page is prepared before this PR is merged.

Approval

The changes are structurally sound and follow the repository's conventions. The addition is appropriately placed in the "For Engineers" section alongside related menu items.


Mention me (@claude) if you'd like me to review any revisions or need additional assistance with this PR.

@pulumi-bot
Copy link
Collaborator

@pulumi-bot
Copy link
Collaborator

@asafashirov asafashirov merged commit 139dd51 into master Dec 2, 2025
8 checks passed
@asafashirov asafashirov deleted the add-guides-nav-item branch December 2, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants