I noticed that fedorapeople.org hides my README.md on website, so I put things on github instead (and updated the link in #1)

Hi,

I will be taking this one.

- Are all the labels not listed in https://docs.fedoraproject.org/en-US/containers/guidelines/creation/ really desired here?

- The help label says 'help="cat /README.md"', In this case, I believe the README.md file should also be provided for the review, and should be placed in /README.md. IMHO, you could just create a help.md file to be shipped in the dist-git root and (see coments below) and drop this label.

- All volumes must be documented in a help.md file, as per https://docs.fedoraproject.org/en-US/containers/guidelines/creation/#_volumes

- It would also be nice to document the exposed ports in the help.md file, as per https://docs.fedoraproject.org/en-US/containers/guidelines/help_file/

For the help.md file, you could also check the changes I am proposing in https://github.com/containers/docs/pull/5

(In reply to Athos Ribeiro from comment #2)
> Hi,
> 
> I will be taking this one.
> 
> - Are all the labels not listed in
> https://docs.fedoraproject.org/en-US/containers/guidelines/creation/ really
> desired here?
About the extra labels I am trying to copy things as in other review requires
- https://bugzilla.redhat.com/show_bug.cgi?id=1439532
- https://bugzilla.redhat.com/show_bug.cgi?id=1438406
Should I delete them?
> 
> - The help label says 'help="cat /README.md"', In this case, I believe the
> README.md file should also be provided for the review, and should be placed
> in /README.md. IMHO, you could just create a help.md file to be shipped in
> the dist-git root and (see coments below) and drop this label.
The fedorapeople.org hides them, I changed to github instead
> 
> - All volumes must be documented in a help.md file, as per
> https://docs.fedoraproject.org/en-US/containers/guidelines/creation/#_volumes
> 
> - It would also be nice to document the exposed ports in the help.md file,
> as per https://docs.fedoraproject.org/en-US/containers/guidelines/help_file/
- They are in README.md, should I place a help.md symlink?
> 
> For the help.md file, you could also check the changes I am proposing in
> https://github.com/containers/docs/pull/5

Okay, changed
 - rename from readme.md to help.md, no longer copy them to container root
 - drop extra labels

Hi Qiyu,

Sorry for the delay here, I realized I had not assigned this to me.

This looks great. Two final details:

- you do not need to copy the help.md file into the image, OSBS will do the magic for you. Just remove the copy line.

- In the help.md file, you need to document which port you are exposing (that does not depend on the config fil, right?)

(In reply to Athos Ribeiro from comment #5)
> Hi Qiyu,
> 
> Sorry for the delay here, I realized I had not assigned this to me.
> 
> This looks great. Two final details:
> 
> - you do not need to copy the help.md file into the image, OSBS will do the
> magic for you. Just remove the copy line.
Done.
> 
> - In the help.md file, you need to document which port you are exposing
> (that does not depend on the config fil, right?)
Actually, it depends on the port specified in the configuration file, I am suggesting to specify port 1080 in this case.

This looks good to me.

I'd rather have that expose section improved, i.e., what are you exposing, and how to use it. The run label example is not referencing the exposed port at all.

I think it is OK to ask users to set their config files to use a specific port so they won't need to change the "run" example or rebuild the image locally to expose a different port.

I will trust you will improve docs as it fits for trojan users, given you definitely know the package better than I do.

Approved.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/container/trojan

Hi Qiyu!

Can we close this now?