feat: add famedly control module in docker image#239
feat: add famedly control module in docker image#239FrenchGithubUser wants to merge 1 commit intomasterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #239 +/- ##
==========================================
- Coverage 80.13% 80.12% -0.01%
==========================================
Files 500 500
Lines 71220 71220
Branches 10703 10703
==========================================
- Hits 57069 57066 -3
- Misses 10906 10908 +2
- Partials 3245 3246 +1 see 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds the famedly-control-synapse module to the Famedly Synapse Docker image build by introducing a new versioned build argument and extending the mod-pack build matrix used by CI.
Changes:
- Add
FCS_VERSIONbuild arg and installfamedly-control-synapseindocker/Dockerfile-famedly. - Introduce
mod020includingfcs-versionand switch the currently-built mod pack list tomod017+mod020.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
docker/Dockerfile-famedly |
Adds a new build arg and pip-install of famedly-control-synapse. |
.ci/scripts/calculate_builds.py |
Adds mod020 with fcs-version and updates which mod packs are built. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| synapse-token-authenticator==${STA_VERSION} \ | ||
| synapse-s3-storage-provider \ | ||
| synapse-invite-checker==${SIC_VERSION} \ | ||
| famedly-control-synapse==${FCS_VERSION} \ |
There was a problem hiding this comment.
famedly-control-synapse==${FCS_VERSION} is installed unconditionally, but ARG FCS_VERSION has no default. If the build arg isn’t provided (or is empty), pip install will fail due to an invalid requirement. Consider either (a) giving FCS_VERSION a safe default and conditionally installing this package only when set, or (b) ensuring every build always passes a valid FCS_VERSION value.
| famedly-control-synapse==${FCS_VERSION} \ | |
| ${FCS_VERSION:+famedly-control-synapse==${FCS_VERSION}} \ |
| "mod020": { | ||
| "sic-version": "0.4.13", | ||
| "sta-version": "0.12.0", | ||
| "fcs-version": "0.1.0", | ||
| }, | ||
| } | ||
|
|
||
| # Adjust this section to decide what gets built and layered on top | ||
| # THIS IS THE SECTION TO EDIT, after you have added the new versions above | ||
| current_mod_packs_to_build = ["mod017", "mod019"] | ||
| current_mod_packs_to_build = ["mod017", "mod020"] | ||
|
|
There was a problem hiding this comment.
current_mod_packs_to_build includes mod017 and mod020, but only mod020 defines fcs-version. Since Dockerfile-famedly now expects FCS_VERSION for installing famedly-control-synapse, the mod017 job will not have a value to pass through the build matrix. Either add fcs-version to mod017 (and any other packs you still build) or adjust the build selection / Dockerfile logic so packs without FCS can still build successfully.
SYN-9