-
-
Notifications
You must be signed in to change notification settings - Fork 5
Update development documentation for setup and workflow #63
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
base: main
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@GabLeRoux has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes consist of extensive revisions to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Documentation
User->>Documentation: Reads DEVELOPMENT.md for setup and workflow
User->>Documentation: Reads README.md for system overview and architecture
Documentation-->>User: Provides structured guidance, diagrams, and procedures
Poem
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. 🪧 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 (
|
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.
Pull Request Overview
This PR updates and improves the documentation for the GameCI Versioning Backend. The changes reorganize DEVELOPMENT.md into clear, step-by-step sections and expand README.md with a comprehensive system overview, including a mermaid diagram and detailed descriptions of CI job workflows.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
README.md | Expanded system overview, CI job workflow explanations, and introduced a mermaid diagram for process visualization. |
DEVELOPMENT.md | Reorganized development setup instructions, added prerequisites, credentials setup, and troubleshooting guidelines. |
Comments suppressed due to low confidence (2)
README.md:16
- [nitpick] Consider aligning the naming in the mermaid diagram with the documentation text; either consistently refer to the component as 'Ingeminator' or include its alias 'repeater' for clarity.
H[Ingeminator] -->|Retry Failed Builds| F
DEVELOPMENT.md:67
- [nitpick] Using a dot in the configuration key (e.g. 'discord.token') is unconventional for environment variable names; if this naming is required by Firebase configuration, consider adding a brief note to clarify its purpose.
export discord.token="your_discord_token"
This is an improved version of #53 so I'll just close previous one. |
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: 3
🧹 Nitpick comments (3)
README.md (2)
30-40
: Specify code block language for CI Job Workflow
The ASCII diagram is in a fenced block without a language specifier. To satisfy markdownlint and improve readability, consider:- ``` + ```text CiJob (e.g., Unity 2022.3.15f1) ├── CiBuild: ubuntu-2022.3.15f1-webgl └── … - ``` + ```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
34-34: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
51-59
: Optional: Simplify nested quotes
In “Ingeminator” you haveThe Ingeminator ("repeater")
. To avoid escaping, you might write:The Ingeminator (alias 'repeater')
DEVELOPMENT.md (1)
123-126
: Grammar tweak for step description
Change “Make changes to code infunctions/src/
” to “Make changes to the code infunctions/src/
” for smoother reading.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DEVELOPMENT.md
(1 hunks)README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
DEVELOPMENT.md
[uncategorized] ~115-~115: The preposition ‘to’ seems more likely in this position.
Context: ...pment Workflow 1. Make changes to code in functions/src/
2. Run the emulator to...
(AI_HYDRA_LEO_REPLACE_IN_TO)
README.md
[grammar] ~61-~61: The word ‘Backup’ is a noun. Do you mean the verb “Back up”?
Context: ...e retry priorities ## Database Backup Backup the Firestore database: ```bash export ...
(SENT_START_BACKUP)
🪛 markdownlint-cli2 (0.17.2)
README.md
34-34: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Deploy
🔇 Additional comments (10)
README.md (4)
3-4
: Well-articulated introduction
This expanded intro succinctly explains the backend’s purpose and main integrations, greatly improving clarity for new contributors.
5-18
: Solid System Overview with Mermaid diagram
The visual depiction of component interactions is clear and immediately conveys the architecture flow. Great addition!
20-28
: Clear Unity Version Ingest section
The enumerated steps lay out the ingest pipeline in a structured way—easy to follow and comprehensive.
73-80
: Great linking to development and contribution guides
Pointing readers toDEVELOPMENT.md
andCONTRIBUTING.md
rounds out the documentation and directs contributors where to go next.DEVELOPMENT.md (6)
1-4
: Comprehensive Guide Title and Overview
The new title and intro clearly set expectations for the development flow—excellent for first-time contributors.
5-12
: Well-defined Prerequisites section
Listing Node.js, Yarn, and Firebase CLI (with version managers) makes onboarding smoother.
13-40
: Clear Setup instructions
Step-by-step commands for installing the Firebase CLI, project dependencies, and running emulators are concise and accurate.
88-108
: Deployment commands are clear
The Firebase login and deploy examples (full and selective) are straightforward and actionable.
110-119
: Firebase Configuration section is thorough
Thefunctions:config:set
commands for rotating or migrating tokens are precise and well documented.🧰 Tools
🪛 LanguageTool
[uncategorized] ~115-~115: The preposition ‘to’ seems more likely in this position.
Context: ...pment Workflow 1. Make changes to code infunctions/src/
2. Run the emulator to...(AI_HYDRA_LEO_REPLACE_IN_TO)
143-148
: Useful Troubleshooting tips
This section addresses the most common gotchas (auth, ports, permissions)—a great aid for local development.
README.md
Outdated
## Database Backup | ||
|
||
The firestore database can be backed up with the following command: | ||
`yarn run backfire export ./export/versioningBackendBackup --project unity-ci-versions --keyFile <PATH_TO_GOOGLE_CLOUD_SERVICE_ACCOUNT_KEYFILE.json>` | ||
Backup the Firestore database: | ||
```bash | ||
export GOOGLE_APPLICATION_CREDENTIALS="/path/to/serviceAccountKey.json" | ||
yarn run backfire export ./export/versioningBackendBackup --project unity-ci-versions --keyFile $GOOGLE_APPLICATION_CREDENTIALS | ||
``` |
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.
🛠️ Refactor suggestion
Unify terminology and fix grammar
- Use the imperative “Back up the Firestore database:” instead of the noun “Backup.”
- The example references
serviceAccountKey.json
, but DEVELOPMENT.md usesservice-account-file.json
. Pick one filename across docs to avoid confusion.
🧰 Tools
🪛 LanguageTool
[grammar] ~61-~61: The word ‘Backup’ is a noun. Do you mean the verb “Back up”?
Context: ...e retry priorities ## Database Backup Backup the Firestore database: ```bash export ...
(SENT_START_BACKUP)
I'll post my feedback here as I follow the Development documentation: When running
I'll now proceed with first logging in and making sure I have Java available in this environment, then continue to give feedback |
Logged in and made sure JAVA was available on path, now get this:
This is due to Mac's port 5000 being used by airplay receiver. I'll continue with a different port and report back |
After changing the hosting port to 5002, the firebase command succeeded:
|
Now that I have the firebase CLI up and running, I do believe I have more issues.
Most notably it can't deploy the cloud functions. |
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 (2)
DEVELOPMENT.md (1)
50-66
: Nitpick: Add definite article for AirPlay Receiver
LanguageTool suggests adding “the” before “AirPlay Receiver” for grammatical clarity.
Apply this diff:-On macOS, port 5000 (used by the Hosting emulator) might be in use by AirPlay Receiver. You can configure a different port in your `firebase.json`: +On macOS, port 5000 (used by the Hosting emulator) might be in use by the AirPlay Receiver. You can configure a different port in your `firebase.json`:🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Possible missing article found.
Context: ...he Hosting emulator) might be in use by AirPlay Receiver. You can configure a different...(AI_HYDRA_LEO_MISSING_THE)
README.md (1)
34-40
: Nitpick: Specify fenced code block language
markdownlint flags the CI Job ASCII diagram as missing a language. Consider usingtext
for better linting:-``` +```text CiJob (e.g., Unity 2022.3.15f1) ├── CiBuild: ubuntu-2022.3.15f1-webgl ├── CiBuild: ubuntu-2022.3.15f1-android ├── CiBuild: windows-2022.3.15f1-webgl └── ... (other baseOS-version-targetPlatform combinations) -``` +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
34-34: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
DEVELOPMENT.md
(1 hunks)README.md
(1 hunks)firebase.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- firebase.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
34-34: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 LanguageTool
DEVELOPMENT.md
[uncategorized] ~50-~50: Possible missing article found.
Context: ...he Hosting emulator) might be in use by AirPlay Receiver. You can configure a different...
(AI_HYDRA_LEO_MISSING_THE)
🔇 Additional comments (20)
DEVELOPMENT.md (11)
7-10
: Comprehensive Prerequisites
The prerequisites section clearly lists all necessary tools and versions.
19-19
: Install Firebase CLI via NPM
Usingnpm i -g firebase-tools
is clear and effective for most environments.
27-33
: Dependency Installation Steps
Installing dependencies at both the repository root and infunctions
is correctly documented.
38-39
: Local Emulator Startup
Documentingfirebase emulators:start
provides a straightforward way to run functions and Firestore locally.
45-47
: Emulator Prerequisites
Calling out Java and Firebase login requirements upfront will help prevent common setup errors.
79-85
: Service Account Configuration
The instructions for settingGOOGLE_APPLICATION_CREDENTIALS
on both macOS/Linux and PowerShell are correct and clear.
93-105
: Integration Environment Variables
Clearly listing Discord, GitHub, and internal token environment variables uses consistent uppercase naming conventions.
115-126
: Deployment Commands
Login and deployment steps (full and partial) are well-documented and include the necessary--only
flag for targeted deploys.
133-140
: Firebase Functions Configuration
Showing both dot-notation for CLI and uppercase env var usage locally clarifies the distinction.
143-154
: Development Workflow
The multi-terminal setup for hot-reloading and testing is adequately described and aligns with common Firebase Function development patterns.
161-170
: Troubleshooting Section
Covering login issues, port conflicts, Java errors, SDK permissions, and integration issues provides helpful quick fixes for developers.README.md (9)
3-3
: Overview Introduction
The expanded introduction effectively summarizes the backend’s purpose and integrations.
7-18
: System Architecture Diagram
The Mermaid diagram clearly illustrates component interactions and data flow.
22-28
: Unity Version Ingest Details
Step-by-step bullet points for version detection, filtering, storage, and notifications are concise and informative.
46-49
: Scheduler Responsibilities
The bullet list accurately highlights job prioritization, retry coordination, and concurrency limits.
53-58
: Ingeminator Section
Describing exponential backoff, retry alerts, and scheduler coordination improves clarity around reliability mechanisms.
62-67
: Database Backup Commands
Providing both export and import commands with environment variable usage offers a complete backup workflow.
69-71
: Restore Workflow
Including the restore command directly after the export instructions ensures users can both back up and restore with the same CLI setup.
75-75
: Development Link
Linking toDEVELOPMENT.md
from the README encourages developers to follow the detailed setup guide.
77-79
: Contributing Section
Pointing toCONTRIBUTING.md
clearly outlines the path for external contributions.
… and avoid conflicts
…d setting up credentials
The part here:
I think you need to build first ( |
I confirm I got further after building! I will now try to create the firebase project and authenticate and report back |
I have setup the firebase project and authenticated, but don't see much of a difference. Note that I also tried to make sure the env variable exists by trying like this: Here's the output from the command now:
|
I'm guessing my empty firebase project doesn't have the admin SDK setup yet. Trying to figure that out now: |
Getting a little stuck here, I think I need to somehow have an existing backend as the firebase project. |
This pull request improves documentation for the GameCI Versioning Backend by restructuring the
DEVELOPMENT.md
file and expanding theREADME.md
to provide a more comprehensive system overview. The changes aim to enhance clarity, usability, and accessibility for developers.Documentation Improvements:
Development Setup and Workflow
DEVELOPMENT.md
: Restructured the document into clear sections, including prerequisites, setup instructions, credentials configuration, and troubleshooting tips. Added details on running Firebase locally, setting up environment variables, and deploying the backend.System Overview and Features
README.md
: Expanded to include a system overview with a mermaid diagram, detailed explanations of Unity version ingestion, CI job workflows, the scheduler, and the Ingeminator. Also added instructions for database backup and restoration.Summary by CodeRabbit