Minor documentation tweaks#2335
Merged
mark9064 merged 4 commits intoInfiniTimeOrg:mainfrom Sep 23, 2025
Merged
Conversation
|
Build size and comparison to main:
|
Contributor
Author
|
Kind ping |
Member
|
Sorry for the delay! I'm away for a couple weeks but I'll try look over this when I can :) |
mark9064
reviewed
Sep 15, 2025
Member
mark9064
left a comment
mark9064
left a comment
There was a problem hiding this comment.
Reads much better now :)
Not a docker expert so I can't review that too closely but all your reasoning looks sound to me (thanks for the great commit messages!)
The first section explains how to clone the repository, the second how to build Infinitime with the docker image, but the details on actually provisioning the image are at the end, despite this step taking place before the build itself. Move the sections into the order in which the steps should be followed.
`docker build` warns of deprecated syntax:
1 warning found (use docker --debug to expand):
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 71)
Update Dockerfile, removing the deprecated syntax usage.
The docker build section points to another page with instructions on how to clone the repository, but this same page already contains these same instructions in the previous section.
The --user argument attempts to map the uid of the user inside the container to the user in the host. This works if docker is running as root, but is docker is running as the current user, then the uid in the container is mapped to a surrogate uid on the host, and this surrogate user does not have permissions to complete the build process. Clarify that the --user flag is only required when running docker as root. It is also likely not required by users using podman as a docker drop-in replacement, since podman always runs in rootless mode.
mark9064
approved these changes
Sep 16, 2025
Member
mark9064
left a comment
mark9064
left a comment
There was a problem hiding this comment.
Looks great! Thanks :)
Reviewer note: will merge in a week or so if no other reviews as this is a doc change only
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See commit messages for finer details.