-
Notifications
You must be signed in to change notification settings - Fork 814
Update Buildah issue template to new version and support podman build #6099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Buildah issue template to new version and support podman build #6099
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a duplicate field in one of the forms, and a couple of questions.
id: storage_conf | ||
attributes: | ||
label: cat /etc/containers/storage.conf output | ||
description: Please copy and paste cat /etc/containers/storage.conf output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On systems where this file isn't present, we'd want to see /usr/share/containers/storage.conf
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated to ask the user to provide the relevant config file and provided the link to https://github.com/containers/storage/blob/main/docs/containers-storage.conf.5.md#files
Let me know what you think
5c329be
to
eee2b2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the thorough review @nalind !
id: storage_conf | ||
attributes: | ||
label: cat /etc/containers/storage.conf output | ||
description: Please copy and paste cat /etc/containers/storage.conf output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated to ask the user to provide the relevant config file and provided the link to https://github.com/containers/storage/blob/main/docs/containers-storage.conf.5.md#files
Let me know what you think
Linking to https://github.com/containers/storage/blob/main/docs/containers-storage.conf.5.md#files looks reasonable. The lack of markup or formatting around the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a couple of spelling changes. Thanks!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nalind, ninja-quokka The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
eee2b2b
to
359270c
Compare
Done thanks! |
.github/ISSUE_TEMPLATE/config.yaml
Outdated
- name: Ask a question | ||
url: https://github.com/containers/buildah/discussions/new | ||
about: Ask a question about Buildah | ||
- name: If your Podman issue is not related to podman build, please report it here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the not
should be dropped here, or perhaps I'm just having a brain-fart.
- name: If your Podman issue is not related to podman build, please report it here | |
- name: If your Podman issue is related to podman build, please report it here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want non-podman build
issues to be filed directly in the podman repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you current suggestion is incorrect. I want this to read as, "if your issue is a general Podman issue, not related to podman build
, then open it in the Podman repo" - If you think the current text is confusing could you please suggest an alternative? Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your example in your last comment makes sense. How's about:
If your issue is a general Podman issue unrelated to podman build
, please open an issue in the Podman repository. If the issue is with the podman build
command, please report it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Tom, Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few scattered nits, otherwise LGTM.
Thanks @ninja-quokka !
359270c
to
6c8a28c
Compare
Thanks for the review @TomSweeneyRedHat, fixed all and left reply on one! |
CI failing on:
|
/packit test |
Signed-off-by: Lewis Denny <[email protected]>
6c8a28c
to
17bb743
Compare
LGTM |
/lgtm |
What type of PR is this?
/kind other
What this PR does / why we need it:
This patch updates the Buildah issue template to the latest format supported by Github and moves more inline with the Podman issue template
The previous issue template handled both Buildah and
podman build
reports in one template, this patch splits into two to be easier to understand.Special notes for your reviewer:
If you like the split issue template for Buildah and podman build, maybe a label would be nice to add to the template like
labels: ["kind/bug", "triage-needed", "kind/buildah"]
orlabels: ["kind/bug", "triage-needed", "kind/podman-build"]
Does this PR introduce a user-facing change?