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

Feature: Further improve issue templates #2073

Merged
merged 10 commits into from
Feb 20, 2022

Conversation

maverick1872
Copy link
Member

@maverick1872 maverick1872 commented Feb 12, 2022

This change-set moves the "Search Terms" input to the bottom of the issue. This means that it won't be the top-most section in the reported issues & feature requests.

Subsequently it adds a link to submit issues/feature requests against WinstonJS/Logform. This URL will direct to the "choose issue type" page when issue templates are implemented on Logform. Until that point it'll just direct to the blank issue form.

label: Additional information
description: >
If you have any additional information for us that you feel will be valuable, please use the field below.
name: Have you encountered an issue?
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why the diffs are against the entire file. This may have to due with the .editorconfig configuration for linebreaks? Will have to inspect these files to see if line-endings changed or not. In the meantime this can be more easily reviewed by enabling the hide whitespace option for the diff itself.

Copy link
Member Author

@maverick1872 maverick1872 Feb 12, 2022

Choose a reason for hiding this comment

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

Verified that line-endings were changed from previous. This is almost guaranteed to be because of the editor config. As such I'm removing that configuration and will let systems/git handle however it sees fit.

Screen Shot 2022-02-12 at 2 26 41 PM

@@ -7,21 +7,13 @@ body:
attributes:
value: >-
This issue form is for reporting bugs only!
- type: input # Search Terms
Copy link
Contributor

@wbt wbt Feb 13, 2022

Choose a reason for hiding this comment

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

Copying comment from winstonjs/logform#149:
The reason search terms came first is to encourage folks to do the search first, before doing all the work to write out a report. If they find a duplicate, they're likely to just comment on that instead of submitting again anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a fair criticism. Whatever resolution is determined I'll re-implement over on logform. Maybe put it second. That way viewers of issues get the most relevant information first but issue creators see it in such a manner that they immediately know they need to fill it out? In the meantime I'll do some further reading on these issue templates to see if there's more configuration available from the resultant template.

Copy link
Member Author

Choose a reason for hiding this comment

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

@wbt reverted this change. It's such a minor thing that it's not worth changing until the issue templates offer more flexibility in how the resultant issue is formatted IMO.

Copy link
Contributor

@DABH DABH left a comment

Choose a reason for hiding this comment

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

LGTM with the latest update. Thank you for this!

Copy link
Contributor

@wbt wbt left a comment

Choose a reason for hiding this comment

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

I don't see any problem with rendering search terms first & wouldn't bother trying to figure out how to change display order, but that's just my perspective.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants