-
Notifications
You must be signed in to change notification settings - Fork 249
chore: rename specs and adrs #2478
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
|
The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).
|
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.
Summary of Changes
Hello @tac0turtle, 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 undertakes a major reorganization of the project's documentation by renaming the specs directory to docs and removing a substantial amount of existing specification and ADR content. This change streamlines the documentation structure, likely in preparation for or as a result of moving detailed documentation to an external repository. It also includes necessary updates to build scripts and internal links to align with the new directory structure.
Highlights
- Documentation Restructuring: The primary
specsdirectory has been renamed todocs, indicating a significant overhaul in the project's documentation organization. - Content Migration and Removal: A large number of detailed specification and Architectural Decision Record (ADR) markdown files, along with their associated
mdbookconfiguration, have been removed. This suggests that this content has been externalized or moved to a separate, dedicated documentation repository. - Build and Link Path Updates: Existing files such as
README.mdandscripts/utils.mkhave been updated to reflect the newdocspath for audit report links and markdown linting ignore patterns, ensuring consistency across the codebase. - Deprecation of Old Documentation Infrastructure: The removal of
mdbookrelated files (e.g.,book.toml,SUMMARY.md) and the impliedspecs-deployaction signifies the deprecation of the previous documentation build and deployment process.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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 renames the specs directory to docs and removes specification files. The changes are consistent with the goal of cleaning up documentation. I've identified potentially incorrect paths in the markdownlint command within scripts/utils.mk following the refactoring. Overall, the changes are positive and align with the PR's objective.
| @golangci-lint run --fix | ||
| @echo "--> Formatting markdownlint" | ||
| @markdownlint --config .markdownlint.yaml --ignore './specs/src/specs/**.md' '**/*.md' -f | ||
| @markdownlint --config .markdownlint.yaml --ignore './docs/src/specs/**.md' '**/*.md' -f |
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.
Similar to line 18, the ignore path is incorrect after renaming specs to docs. The docs/src/specs directory does not exist. Update the path to reflect the new directory structure, or remove the --ignore flag if it's no longer needed.
@markdownlint --config .markdownlint.yaml --ignore './docs/src/**.md' '**/*.md' -f
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2478 +/- ##
==========================================
- Coverage 73.11% 73.05% -0.07%
==========================================
Files 68 68
Lines 6542 6542
==========================================
- Hits 4783 4779 -4
- Misses 1361 1364 +3
- Partials 398 399 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Claude finished @tac0turtle's task —— View job Code Review CompleteTodo List:
Review Findings✅ Strengths
🐛 Bugs IdentifiedCritical Issue in Line 34: The markdownlint ignore path is incorrect after the directory rename: @markdownlint --config .markdownlint.yaml --ignore './docs/src/specs/**.md' '**/*.md' -fProblem: The path
Impact: This will cause the markdownlint command to fail or behave unexpectedly since it's trying to ignore a non-existent directory. Recommendation: Either:
📊 Code Quality Assessment
🔒 Security Considerations
⚡ Performance Considerations
🧪 Test Coverage
SummaryThis is a well-intentioned cleanup PR that successfully moves documentation to a better directory structure. However, there's one critical bug that needs fixing: the markdownlint ignore path in Required Action: Update or remove the outdated markdownlint ignore path to prevent CI failures. |
Overview
rename specs to docs and remove things that were added to the docs repo. This pr also removes the specs-deploy action as we will no longer link to the specs page