[Bug 1862054] New: Container Review Request - trojan - An unidentifiable mechanism that helps you avoid censorship
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
Bug ID: 1862054 Summary: Container Review Request - trojan - An unidentifiable mechanism that helps you avoid censorship Product: Fedora Container Images Version: rawhide Status: NEW Component: Container Review Assignee: nobody@fedoraproject.org Reporter: yanqiyu01@gmail.com QA Contact: extras-qa@fedoraproject.org CC: container-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Container Build Info URL: https://yanqiyu.fedorapeople.org/trojan-container/Dockerfile Description: An unidentifiable mechanism that helps you avoid censorship. Fedora Account System Username: yanqiyu
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |athoscribeiro@gmail.com Doc Type|--- |If docs needed, set a value Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
Qiyu Yan yanqiyu01@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Comment|0 |updated
--- Comment #0 has been edited ---
Container Build Info URL: https://yanqiyu.fedorapeople.org/trojan-container/Dockerfile Description: An unidentifiable mechanism that helps you avoid censorship. Fedora Account System Username: yanqiyu
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #1 from Qiyu Yan yanqiyu01@gmail.com --- I noticed that fedorapeople.org hides my README.md on website, so I put things on github instead (and updated the link in #1)
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #2 from Athos Ribeiro athoscribeiro@gmail.com --- 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/#_volume...
- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #3 from Qiyu Yan yanqiyu01@gmail.com --- (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/#_volume...
- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #4 from Qiyu Yan yanqiyu01@gmail.com --- Okay, changed - rename from readme.md to help.md, no longer copy them to container root - drop extra labels
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |athoscribeiro@gmail.com
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #5 from Athos Ribeiro athoscribeiro@gmail.com --- 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?)
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #6 from Qiyu Yan yanqiyu01@gmail.com --- (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.
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #7 from Athos Ribeiro athoscribeiro@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #8 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/container/trojan
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
--- Comment #9 from Athos Ribeiro athoscribeiro@gmail.com --- Hi Qiyu!
Can we close this now?
https://bugzilla.redhat.com/show_bug.cgi?id=1862054
Qiyu Yan yanqiyu01@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2020-08-21 23:51:18
container-review@lists.fedoraproject.org
-
bugzilla＠redhat.com