squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.67k stars 1.48k forks source link

Bug report template: various tweaks #3829

Closed jrfnl closed 1 year ago

jrfnl commented 1 year ago

Description

Suggested changelog entry

N/A

Related issues/external references

N/A

Types of changes

gsherwood commented 1 year ago

Github's preview of the file isn't rendering that table at all. I'm not sure if there is a syntax error there.

jrfnl commented 1 year ago

Github's preview of the file isn't rendering that table at all. I'm not sure if there is a syntax error there.

Thanks for catching that. That must be an issue with the GitHub flavoured markdown being less tolerant of table formats, but it should be good now.

gsherwood commented 1 year ago

Thanks. I've merged it in.

When trying out the new template the table layout in text has been messed up for me, which makes it hard to complete the report. I think it's probably just the font but switching it to a simple list might be more compatible.

This is what I see with default settings:

Screenshot 2023-05-25 at 9 09 52 am
jrfnl commented 1 year ago

When trying out the new template the table layout in text has been messed up for me, which makes it hard to complete the report. I think it's probably just the font but switching it to a simple list might be more compatible.

@gsherwood That's the downside of the change I made to get the table working without header. The alternative would be to bring the table back to the original format (which should display less messily), but add table headers. Just means we'd have to think of what the headers should be....

fredden commented 1 year ago

Might something like this work? https://github.com/fredden/yaml-issue-template-test/issues/new?template=bug_report.yaml https://github.com/fredden/yaml-issue-template-test/blob/main/.github/ISSUE_TEMPLATE/bug_report.yaml You're welcome to experiment with that repository; I'll delete it after the discussion here has concluded.

Ref: https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-issue-forms

jrfnl commented 1 year ago

@fredden I'm not really a fan of the YAML templates as I've found them to be too restrictive/limiting in most cases.

With the markdown templates, people can easily add an extra section or two or a detail fold-out with log information etc. With the YAML templates they can't and they'd have to add that information in follow-up comments, which doesn't necessarily make it clearer what the extra information belongs with.

fredden commented 1 year ago

Yes, I've found it difficult to put correct details in YAML style templates too. I hoped that the 'additional context' box at the bottom would suffice for this.

I was expecting to find some syntax to create a form similar to the 'affected products' section of the security vulnerability report, but didn't find this style described in the documentation.

Screen-shot of vulnerability report form ![Screenshot_2023-05-26_16-25-52](https://github.com/squizlabs/PHP_CodeSniffer/assets/334786/ade45421-1eed-4101-98f9-492b9721c1b7)

Back to / sticking with the markdown template:

Markdown doesn't need a lot of white-space around the table syntax. I wonder if removing that would increase the usability. Making the text line up in a mono-space font is nice, but the input box in GitHub for issues doesn't use a mono-space font by default.

eg, this table renders just fine:

| | |
| --- | --- |
| Operating System | Debian 11
| PHP version | 8.2
| Standard | PSR12
Operating System Debian 11
PHP version 8.2
Standard PSR12
jrfnl commented 1 year ago

That's weird as that is the table format I had originally when I first pulled this PR... 🤷🏻‍♀️

fredden commented 1 year ago

I think it needs the trailing pipe on the first row to work properly.

| |
| --- | ---
| Operating System | Debian 11
| PHP version | 8.2
| Standard | PSR12
Operating System Debian 11
PHP version 8.2
Standard PSR12
| | |
| --- | ---
| Operating System | Debian 11
| PHP version | 8.2
| Standard | PSR12
Operating System Debian 11
PHP version 8.2
Standard PSR12
jrfnl commented 1 year ago

@fredden That sounds like gfm (GitHub flavoured Markdown) weirdness to me as that shouldn't be needed based on the Markdown specs IIRC, but thank you for figuring it out. Nice find!

Do you want to send in a follow up PR to fix up that table format ?