Skip to content

Conversation

@Fryguy
Copy link
Member

@Fryguy Fryguy commented Jan 12, 2024

@ManageIQ/core-admins Please review.

My goal is to eventually also remove the bespoke Issue and PR templates, and ideally only have this consistent list, except where we need to override it.

I tried really hard to keep it short and sweet. As a contributor, I really can't stand walls-of-words, especially they make me break it down into separate "describe the problem/what happened/what did you expect to happen" sections.

As a reviewer I can't stand seeing all the boilerplate, cause it just gets in the way, especially when the author doesn't remove the boilerplate help text. So I listed them here as actual HTML comments, so if contributors do leave them in, then at least the final result we look at doesn't have all that stuff.


TODO: Remove Issue/PR templates from other repos

@Fryguy Fryguy requested a review from a team January 12, 2024 22:45
@Fryguy Fryguy added the documentation Improvements or additions to documentation label Jan 12, 2024
<!--
3. Describe your environment, including
- Version
- Deployment environment (e.g. VMware appliance, EC2 appliance, Containers, etc)
Copy link
Member Author

@Fryguy Fryguy Jan 12, 2024

Choose a reason for hiding this comment

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

This part was interesting. Since this is at the org level, this template will apply to everything (the ManageIQ appliancation, gems, build repos, docs, etc). I wasn't sure how to present this for all of those, so I'm up for some help here. The goal is short and sweet, so I don't want to put too much to be annoying.

Copy link
Member

@jrafanie jrafanie Jan 17, 2024

Choose a reason for hiding this comment

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

Is there a magic variable for current repo that we could put context specific information? For gems like kubeclient, optimist, linux_admin, etc. this template could be confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

No it's just a straight text file. It we want to override we have to create a file in the repo itself

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a few of us can jump on a call, merge this, and see how it looks for several repos and add any overrides for repos where this template would be confusing, such as: kubeclient, optimist, linux_admin.

contact_links:
- name: ":question: Question / Support"
url: https://github.com/orgs/ManageIQ/discussions
about: Please ask and answer questions here.
Copy link
Member Author

Choose a reason for hiding this comment

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

The goal here was to push people asking questions to the discussions instead of opening actionable tickets. It should show up at the bottom of the chooser list, but it's hard for me to know until we merge it.

-->

<!--
3. Tell @miq-bot to label this PR with an appropriate scope label (bug, enhancement, etc)
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for this item to be different from the issue templates?

Copy link
Member Author

@Fryguy Fryguy Jan 18, 2024

Choose a reason for hiding this comment

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

In the issue template we know ahead of time if it's a bug or enhancement, since those are the 2 choices we offer. For PRs we don't know what it's for.

Comment on lines 15 to 17
3. Describe your environment, including
- Version
- Deployment environment (e.g. VMware appliance, EC2 appliance, Containers, etc)
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
3. Describe your environment, including
- Version
- Deployment environment (e.g. VMware appliance, EC2 appliance, Containers, etc)
3. Describe your environment, including
- Version
- Environment
- For ManageIQ: VMware appliance, EC2 appliance, Monolithic container, Kubernetes operator, etc
- For gems: `gem env`, `bundle env`, `ruby -v`, etc

-->

<!--
2. Describe the steps to reproduce, including any log snippets that will help diagnose.
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
2. Describe the steps to reproduce, including any log snippets that will help diagnose.
2. Describe the steps to reproduce, including any log snippets and stack traces that will help diagnose.

@Fryguy Fryguy force-pushed the add_default_templates branch from 4b11b33 to cc166d3 Compare January 25, 2024 17:15
Fryguy added a commit to Fryguy/manageiq that referenced this pull request Jan 25, 2024
@bdunne bdunne merged commit 9f579a1 into ManageIQ:master Jan 25, 2024
@bdunne bdunne assigned bdunne and unassigned jrafanie Jan 25, 2024
@Fryguy Fryguy deleted the add_default_templates branch January 25, 2024 17:19
Fryguy added a commit to Fryguy/manageiq-pods that referenced this pull request Jan 25, 2024
Fryguy added a commit to Fryguy/manageiq-ui-classic that referenced this pull request Jan 25, 2024
Fryguy added a commit to Fryguy/rbvmomi2 that referenced this pull request Jan 25, 2024
bdunne added a commit to ManageIQ/manageiq that referenced this pull request Jan 25, 2024
bdunne added a commit to ManageIQ/manageiq-pods that referenced this pull request Jan 25, 2024
bdunne added a commit to ManageIQ/manageiq-ui-classic that referenced this pull request Jan 25, 2024
agrare added a commit to ManageIQ/rbvmomi2 that referenced this pull request Jan 25, 2024
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.

3 participants