Improve Development Documentation #53
Conversation
| ``` | ||
| export internal.token="my_internal_token" | ||
| ```bash | ||
| export INTERNAL_TOKEN="your_internal_token" |
There was a problem hiding this comment.
I found it weird to have dots in these values, but this is probably going to break things. 🤔 Is this a special syntax for firebase functions configuration?
I would appreciate if someone could confirm this one so we can go forward with merging the rest of it 🙏
There was a problem hiding this comment.
I think I pretty much confirmed this by reading the code.
functions/src/api/testFunction.ts
12:const internalToken = defineSecret('INTERNAL_TOKEN');
functions/src/api/reportNewBuild.ts
14:const internalToken = defineSecret('INTERNAL_TOKEN');
functions/src/api/reportPublication.ts
12:const internalToken = defineSecret('INTERNAL_TOKEN');
functions/src/api/reportBuildFailure.ts
11:const internalToken = defineSecret('INTERNAL_TOKEN');
DEVELOPMENT.md
147:export INTERNAL_TOKEN="your_internal_token"
I'm quite sure the syntax when working locally with export is to use INTERNAL_TOKEN instead of internal.token.
| ``` | ||
| - **Start Specific Emulators Only:** To run specific components (e.g., `functions` only): | ||
| ```bash | ||
| firebase emulators:start --only functions |
There was a problem hiding this comment.
I'm not sure if this will work, but working with firebase emulators is most likely a good approach to working locally with this project. Feedback would be appreciated here.
There was a problem hiding this comment.
The thing is that in order to create a proper dev workflow you'd have to mock the dispatching github actions (for the building of images) from the cronjob, or skip the cronjob altogether (at which point the emulators don't do much at all).
|
I added some comments which I forgot to hit submit as a self review. I would appreciate if someone can give it a try and confirm if the commands are ok. I will give it a try too shortly |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe changes involve a comprehensive restructuring and enhancement of the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant DevelopmentGuide
participant README
Developer->>DevelopmentGuide: Access Development Guide
DevelopmentGuide-->>Developer: Provides setup and deployment instructions
Developer->>README: Access README
README-->>Developer: Provides overview and functionalities
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (11)
README.md (7)
1-9: Consider enhancing the introduction with badges and quick links.The introduction provides a clear overview, but could be more informative with:
- Status badges (build status, version, license)
- Quick links to key sections (setup, deployment, contributing)
- Link to the live service or demo if available
11-18: Consider adding a data structure example.The Unity Version Ingest section clearly explains the process. Consider adding an example of how the version data is structured in Firestore to help developers understand the data model.
22-24: Enhance the Docker Version Ingest section with more details.Consider:
- Simplifying "in conjunction with" to "alongside"
- Adding details about:
- How often versions are checked
- Where the versions are sourced from
- Example of the version format
🧰 Tools
🪛 LanguageTool
[style] ~24-~24: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...cker images. These versions are tracked in conjunction with Unity versions, ensuring Docker images ...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
34-38: Add language specification to the code block.Specify the language for the code block to enable syntax highlighting.
- ``` + ```text🧰 Tools
🪛 Markdownlint
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
35-37: Clarify the version placeholder usage.Consider adding a note explaining what
<version>represents in these examples and its possible values.
48-50: Documentation needed for the Ingeminator section.The section indicates that documentation is pending. Consider adding:
- Retry logic and conditions
- Maximum retry attempts
- Failure handling strategy
Would you like me to help draft detailed documentation for the Ingeminator section?
56-72: Add safety warnings for database operations.Consider adding important warnings:
- Backup verification steps
- Production data handling precautions
- Required permissions for backup/restore operations
DEVELOPMENT.md (4)
5-27: Maintain consistent unordered list style in TOC.The table of contents uses asterisks for list items. For consistency with the rest of the document and markdown best practices, consider using dashes instead.
-* [Development Guide](#development-guide) - * [Prerequisites](#prerequisites) - * [Setup](#setup) +- [Development Guide](#development-guide) + - [Prerequisites](#prerequisites) + - [Setup](#setup)🧰 Tools
🪛 LanguageTool
[grammar] ~17-~17: The operating system from Apple is written “macOS”.
Context: ...etting-up-credentials) * Linux / MacOS * [Windows (Power...(MAC_OS)
109-109: Update OS name to use correct spelling.The Apple operating system should be written as "macOS" instead of "MacOS" in the section headers.
-#### Linux / MacOS +#### Linux / macOSAlso applies to: 115-115
🧰 Tools
🪛 LanguageTool
[grammar] ~109-~109: The operating system from Apple is written “macOS”.
Context: ...to the path of this file. #### Linux / MacOS ```bash export GOOGLE_APPLICATION_CRED...(MAC_OS)
158-162: Consider adding validation steps for config variables.While the configuration commands are correct, it would be helpful to add instructions for verifying that the variables were set correctly:
firebase functions:config:set discord.token="your_discord_token" firebase functions:config:set github.client-secret="your_github_app_client_secret" firebase functions:config:set github.private-key="your_github_app_private_key" firebase functions:config:set internal.token="your_internal_token" + +# Verify the configuration +firebase functions:config:get
186-190: Consider expanding troubleshooting section.The troubleshooting section could be more helpful with specific examples of common issues and their solutions, such as:
- Error messages when environment variables are missing
- Common Firebase emulator issues
- Debugging integration problems
Would you like me to help generate more detailed troubleshooting examples?
🧰 Tools
🪛 Markdownlint
188-188: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
189-189: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- DEVELOPMENT.md (1 hunks)
- README.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
DEVELOPMENT.md
[grammar] ~17-~17: The operating system from Apple is written “macOS”.
Context: ...etting-up-credentials) * Linux / MacOS * [Windows (Power...(MAC_OS)
[grammar] ~109-~109: The operating system from Apple is written “macOS”.
Context: ...to the path of this file. #### Linux / MacOS ```bash export GOOGLE_APPLICATION_CRED...(MAC_OS)
README.md
[style] ~24-~24: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...cker images. These versions are tracked in conjunction with Unity versions, ensuring Docker images ...(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
🪛 Markdownlint
DEVELOPMENT.md
33-33: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
34-34: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
169-169: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
174-174: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
179-179: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
188-188: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
189-189: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
README.md
34-34: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
🔇 Additional comments (4)
README.md (1)
76-84: LGTM! Clear and welcoming development and contribution sections.The sections provide appropriate links to detailed documentation and encourage contributions effectively.
DEVELOPMENT.md (3)
29-69: LGTM! Clear and accurate setup instructions.The setup instructions are well-structured and follow the agreed-upon practice of using npm for global installations while using yarn for project dependencies.
🧰 Tools
🪛 Markdownlint
33-33: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
34-34: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
70-99: LGTM! Comprehensive deployment guide.The deployment instructions are clear and include both full and selective deployment options with proper examples.
125-153: LGTM! Clear environment variable configuration.The environment variables section provides clear instructions for setting up all necessary integrations.
This pull request includes significant updates to the documentation, particularly enhancing the
DEVELOPMENT.mdguide and theREADME.mdfile. The changes aim to provide clearer instructions and a more comprehensive overview of the project's setup, deployment, and functionality.Documentation Enhancements:
Development Guide:
# Developmentto# Development Guideand added a detailed table of contents.README Overview:
backfiretool.DEVELOPMENT.mdfor further development instructions.I went forward and tried to improve the docs. I wrote this after seing a user mentionning they couldn't find the docs on how to work with this project after looking at the readme. There is now a link to
DEVELOPMENT.mdat the end of the ReadMe.You can see the results here:
Summary by CodeRabbit
DEVELOPMENT.mdto improve clarity and usability, including new sections on prerequisites, setup, deployment, and troubleshooting.README.mdto provide a clearer overview of the GameCI Versioning Backend's functionalities, including detailed descriptions of automation processes and integration capabilities.README.md.