Skip to content
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

[github]: add new Frontend_Report / Issue_Report & optimize config.yml #226

Merged
merged 14 commits into from
Nov 19, 2024

Conversation

MickLesk
Copy link
Member

Description

  1. Add new Frontend_Report for Website
  2. Optimize Issue Report
  3. Optimize Issue Overview

Old:
image

New:
image

Type of change

Please check the relevant option(s):

  • Bug fix (non-breaking change that resolves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (a fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts.)

Prerequisites

The following efforts must be made for the PR to be considered. Please check when completed:

  • Self-review performed (I have reviewed my code, ensuring it follows established patterns and conventions)
  • Testing performed (I have tested my changes, ensuring everything works as expected)
  • Documentation updated (I have updated any relevant documentation)

@MickLesk MickLesk requested a review from a team as a code owner November 13, 2024 15:55
@remz1337
Copy link
Contributor

Not sure about the documentation one. I checked the link the PR and it redirects to frontend issue?

@MickLesk
Copy link
Member Author

Not sure about the documentation one. I checked the link the PR and it redirects to frontend issue?

This is currently intended because the general documentation for LXCs/VMs and other scripts can be found on the website. (CPU/RAM/login etc )

BramSuurdje
BramSuurdje previously approved these changes Nov 13, 2024
Copy link
Contributor

@havardthom havardthom left a comment

Choose a reason for hiding this comment

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

Have some comments, but eating dinner atm, will add after

@MickLesk
Copy link
Member Author

Have some comments, but eating dinner atm, will add after

Take your time, I'm out for today :-) If then I'll take a look at it tomorrow.

Copy link
Contributor

@havardthom havardthom left a comment

Choose a reason for hiding this comment

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

Quite a few comments here, honestly I just think I like the old templates better. This adds a lot and feels a little bloated to me. In my eyes we have two things which are user-facing: the scripts and a website. Having one template for each feels simpler and more consistent for me.

.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Show resolved Hide resolved
.github/ISSUE_TEMPLATE/frontend_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/frontend_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/frontend_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/frontend_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/frontend_report.yml Show resolved Hide resolved
@remz1337
Copy link
Contributor

Really great review @havardthom, I agree with everything

@MickLesk
Copy link
Member Author

I absolutely do not agree with every comment. Some comments are also in the existing template from tteck. Somehow it seems like I'm being portrayed as more stupid ^^

About the wording, okay but you don't have to optimize every single sentence. It's already hard "educating", like a teacher. But as far as I'm concerned, I can look through it tomorrow.

@remz1337
Copy link
Contributor

@MickLesk please don't take this personally. I think you have been doing an excellent job as maintainer so far, don't downplay yourself.

Me agreeing with his review doesn't mean I'm right either ;) I'm just providing my thoughts. This project now being community driven means we may need more time to come to an agreement. Would be great to hear from other contributors too (@community-scripts/contributor)

@MickLesk MickLesk added documentation A change to documentation enhancement New feature or request nice to have A change that is a nice to have labels Nov 13, 2024
@havardthom
Copy link
Contributor

@MickLesk I'm sorry if I come off as harsh, I'm just voicing my opinions and it is fine to disregard them if people disagree. I do a lot of nitpicking but I feel it is important to keep the consistency and simplicity tteck lived by.
I absolutely don't think or want to portray you as stupid, I think you do great work for the community and I really appreciate all of it, as I'm sure the rest of the team also does.

@Mellowlynx
Copy link
Contributor

@MickLesk I could not agree more with @havardthom last message.
The consistency and simplicity tteck used is indeed important, and it can be nice to have someone who is nitpicking.
As it shows's respect and real thought about the subject. (most of the time)
We all see points different, but let's use the feedback to improve overall and try to see someone else's perspective.

@Mellowlynx
Copy link
Contributor

As it's getting late, I will be happy to also check and check the feedback sometime tomorrow.

switch general to other

Co-authored-by: Håvard Gjøby Thom <[email protected]>
MickLesk and others added 11 commits November 14, 2024 09:10
remove title

Co-authored-by: Håvard Gjøby Thom <[email protected]>
remove label

Co-authored-by: Håvard Gjøby Thom <[email protected]>
update text for questions and help

Co-authored-by: Håvard Gjøby Thom <[email protected]>
harmonize with readme (discord)

Co-authored-by: Håvard Gjøby Thom <[email protected]>
harmonize title for Frontend

Co-authored-by: Håvard Gjøby Thom <[email protected]>
harmonize text

Co-authored-by: Håvard Gjøby Thom <[email protected]>
update label

Co-authored-by: Håvard Gjøby Thom <[email protected]>
remove title

Co-authored-by: Håvard Gjøby Thom <[email protected]>
@github-actions github-actions bot added the maintenance Code maintenance or general upkeep of the project label Nov 19, 2024
@MickLesk
Copy link
Member Author

MickLesk commented Nov 19, 2024

So ive updated smart little things.
I think it looks good, is clearly laid out and contains the most important functions.
The only thing that could be made smarter is the text under the title of the issue report.

Feedback on this is welcome!

You can test it here:
https://github.com/MickLesk/Proxmox_DEV/issues/new/choose

@BramSuurdje
Especially for you, you will “work” the most with it (Frontend-Report). Is the front-end report good, or is it missing something, or is it too much?

@MickLesk MickLesk self-assigned this Nov 19, 2024
@MickLesk MickLesk removed the nice to have A change that is a nice to have label Nov 19, 2024
Copy link
Contributor

@havardthom havardthom left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for improving

@MickLesk MickLesk removed the request for review from Mellowlynx November 19, 2024 18:04
@MickLesk MickLesk merged commit 9148ea1 into main Nov 19, 2024
3 checks passed
@MickLesk MickLesk deleted the MickLesk_Dev branch November 19, 2024 20:38
@community-scripts community-scripts locked and limited conversation to collaborators Nov 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation A change to documentation enhancement New feature or request maintenance Code maintenance or general upkeep of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants