docs: add formal proposal process and contribution guidelines#83
docs: add formal proposal process and contribution guidelines#83
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive upgrade to the UMS (Unified Module System) from v1.0 to v2.0, introducing TypeScript support, conflict-aware module registry, and a redesigned architecture focused on pure functional operations. The changes establish TypeScript as the primary format while maintaining backward compatibility and provide enhanced development experience with modular exports.
- TypeScript-first architecture with .module.ts and .persona.ts support
- Conflict-aware ModuleRegistry with configurable resolution strategies
- Complete rewrite of ums-lib with pure functional API design
- MCP (Model Context Protocol) server implementation
Reviewed Changes
Copilot reviewed 85 out of 90 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/ums-mcp/src/index.ts | MCP server entry point for Claude Desktop integration |
| packages/ums-lib/src/ | Complete v2.0 rewrite with pure functions, conflict registry, and TypeScript support |
| packages/copilot-instructions-cli/src/utils/ | TypeScript loader and enhanced CLI utilities |
| modules.config.yml | Updated conflict strategy to "error" for stricter validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * Provides error classes and utility functions | ||
| */ | ||
|
|
||
| export * from './errors.js'; |
There was a problem hiding this comment.
The utils/index.ts file only exports errors but the transforms.js utility is not exported. This creates inconsistent access patterns where moduleIdToExportName must be imported directly from the main package but other utilities are available through the utils submodule.
| export * from './errors.js'; | |
| export * from './errors.js'; | |
| export * from './transforms.js'; |
| @@ -0,0 +1,441 @@ | |||
| import { describe, it, expect } from 'vitest'; | |||
| import { validatePersona } from '../validation/persona-validator.js'; | |||
There was a problem hiding this comment.
The test file imports validatePersona but the file is named persona-parser.test.ts and should be testing the parser functions. The import suggests this test file is misnamed or testing the wrong functionality.
Summary of ChangesHello @synthable, 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 significantly enhances the project's governance and technical foundation by introducing a formal proposal process and comprehensive contribution guidelines. Concurrently, it marks a major architectural shift to UMS v2.0, embracing a TypeScript-first development model across the core library and CLI tools. These changes aim to foster a more structured, collaborative, and robust development environment, improving both contributor experience and the overall quality of the codebase. Highlights
Ignored Files
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.
Code Review
This is an extensive and well-thought-out pull request that introduces a formal proposal process, comprehensive contribution guidelines, and a major architectural refactoring to UMS v2.0. The move to a TypeScript-first approach and the separation of the core library (ums-lib) from I/O operations in the CLI are excellent improvements for maintainability and testability. The new documentation is very thorough. I have a few suggestions regarding broken links in the new documentation and a potential issue with module ID validation, but overall this is a fantastic contribution to the project.
| } from '../../types/index.js'; | ||
| import { ValidationError as ValidationErrorClass } from '../../utils/errors.js'; | ||
|
|
||
| const MODULE_ID_REGEX = /^[a-z0-9][a-z0-9-]*(?:\/[a-z0-9][a-z0-9-]*)*$/; |
There was a problem hiding this comment.
The MODULE_ID_REGEX on this line seems too lenient. It allows module IDs that do not start with one of the four valid tiers (foundation, principle, technology, execution), for example, it would validate an ID like foo/bar. Given that the tier system is a core concept of UMS, the regex should probably enforce the presence of a valid tier at the beginning of the ID. A stricter regex might be more appropriate to ensure all module IDs are correctly namespaced within the four-tier system.
| - You MUST commit only your work. Do NOT include changes from other team members or unrelated modifications. | ||
| - You MUST commit your changes in logical chunks after completing a task. | ||
| - You MUST run lints and tests and ensure all pass before committing. | ||
| - You MUST ensure your code is well-documented and follows the project's coding standards. | ||
| - You MUST write unit tests for new features or bug fixes. |
There was a problem hiding this comment.
There appears to be some duplication of contributor guidelines in this file. This block is very similar to the Contributor Requirements section at the end of the file (lines 261-266). The comment at line 257 (<!-- Consolidated contributor requirements (replaces duplicated block) -->) suggests a consolidation was intended. To improve the clarity and maintainability of this guide, it would be best to remove this duplicated block and rely on the consolidated section at the end.
| - Technology modules (language/framework specific) | ||
| - Execution modules (procedures and playbooks) | ||
|
|
||
| See [Module Authoring Guide](docs/unified-module-system/12-module-authoring-guide.md) for details. |
There was a problem hiding this comment.
| - `npm run test`: Run tests. | ||
| - `npm run lint`: Lint the codebase. | ||
| - `npm run format`: Format the codebase. | ||
| For a deep dive into the Unified Module System, advanced features, and configuration, please read our **[Comprehensive Guide](./docs/comprehensive_guide.md)**. |
docs/README.md
Outdated
| - **[User Guide](./USER_GUIDE.md):** A comprehensive guide to using the `copilot-instructions-cli` tool. | ||
| - **[Unified Module System (UMS)](./unified-module-system/index.md):** The core documentation for the UMS specification, module authoring, and philosophy. | ||
| - **[FAQ](./FAQ.md):** Frequently asked questions about the project. |
There was a problem hiding this comment.
Several links in this new documentation index appear to be broken as the corresponding files or directories are not part of this pull request. For example, USER_GUIDE.md is being removed, and files like unified-module-system/index.md and FAQ.md are not present. Please verify these links and add the necessary files or update the paths to ensure the documentation is navigable.
Add comprehensive documentation for the project's proposal process and contribution guidelines to support community contributions and ensure consistent quality across the codebase. Changes: - Add CONTRIBUTING.md with detailed contribution guidelines - Code of conduct and development workflow - Branch strategy and commit message conventions - Pull request process and review guidelines - Testing requirements and coding standards - Package-specific guidelines for each package - Add formal proposal process documentation - docs/proposal-process.md: Complete proposal lifecycle - docs/proposal-quick-start.md: 5-minute quick start guide - docs/spec/proposals/TEMPLATE.md: Proposal template - Add GitHub issue templates for proposals - .github/ISSUE_TEMPLATE/proposal.md: New proposal submission - .github/ISSUE_TEMPLATE/proposal-feedback.md: Feedback tracking Proposal Process Features: - Lightweight for small changes, structured for significant changes - 7-day minimum review period for major proposals - Clear acceptance criteria and decision-making process - Integration with GitHub issues and pull requests - Support for iterative refinement based on feedback This establishes a clear path for community members to propose and implement features, architectural changes, and specification updates while maintaining project quality and coherence.
4b06b29 to
01b933d
Compare
Summary
This PR introduces a comprehensive proposal process and contribution guidelines to support community contributions and ensure consistent quality across the codebase.
What's New
📝 Contribution Guidelines (CONTRIBUTING.md)
Complete guide for contributing to the project:
🎯 Proposal Process
Two new documentation files establish a formal process for significant changes:
docs/proposal-process.md - Complete proposal lifecycle:
docs/proposal-quick-start.md - 5-minute quick start:
📋 Templates and Issue Forms
docs/spec/proposals/TEMPLATE.md - Structured proposal template:
GitHub Issue Templates:
.github/ISSUE_TEMPLATE/proposal.md- New proposal submission.github/ISSUE_TEMPLATE/proposal-feedback.md- Feedback trackingKey Features
Lightweight Yet Structured
Community-Friendly
Integration with GitHub
Files Changed
New Files (1,834 lines added)
Updated Files
Benefits
Testing
Checklist
Next Steps
After merging, contributors can:
This establishes the foundation for a healthy, collaborative open-source project! 🚀