Skip to content

Conversation

WhyNotHugo
Copy link
Contributor

See commit messages for finer details.

Copy link

github-actions bot commented Jul 23, 2025

Build size and comparison to main:

Section Size Difference
text 380132B 0B
data 944B 0B
bss 22544B 0B

Run in InfiniEmu

@mark9064 mark9064 added the documentation Improvements or additions to documentation label Aug 5, 2025
@WhyNotHugo
Copy link
Contributor Author

Kind ping

@mark9064
Copy link
Member

mark9064 commented Sep 2, 2025

Sorry for the delay! I'm away for a couple weeks but I'll try look over this when I can :)

@mark9064 mark9064 self-requested a review September 2, 2025 17:12
Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Member

@mark9064 mark9064 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks :)

Reviewer note: will merge in a week or so if no other reviews as this is a doc change only

@mark9064 mark9064 added this to the 1.16.0 milestone Sep 16, 2025
@mark9064 mark9064 merged commit 957ba59 into InfiniTimeOrg:main Sep 23, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants