Skip to content

Added github issues templates #1952

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

Merged
merged 31 commits into from
Apr 15, 2025
Merged

Added github issues templates #1952

merged 31 commits into from
Apr 15, 2025

Conversation

Bashamega
Copy link
Contributor

closes #1032

@saschanaz
Copy link
Contributor

saschanaz commented Mar 30, 2025

Sounds nice to have modern templates. IMO almost everything filed in this repo is about typing issue though, so I doubt we need to have multiple templates. We can start with one and let people use blank template if it doesn't fit.

@Bashamega Bashamega marked this pull request as ready for review March 30, 2025 14:34
@Bashamega
Copy link
Contributor Author

Sounds nice to have modern templates. IMO almost everything filed in this repo is about typing issue though, so I doubt we need to have multiple templates. We can start with one and let people use blank template if it doesn't fit.

Sure

@Bashamega
Copy link
Contributor Author

Done @saschanaz

@HolgerJeromin
Copy link
Contributor

Should we add here the faq about two needed engines:
https://github.com/microsoft/TypeScript-DOM-lib-generator?tab=readme-ov-file#when-the-type-is-missing

attributes:
label: "Steps to Reproduce"
description: "List steps to reproduce the issue"
placeholder: "Step 1, Step 2, Step 3..."
Copy link
Contributor

Choose a reason for hiding this comment

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

For this repo we don't really need repro steps, a playground link would be the best.

issue_templates:
- name: 🔍 Typings Inconsistency
description: Report inconsistencies between TypeScript's DOM typings and browser behavior.
labels: [inconsistency]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe reuse an existing label - lib.d.ts bug?

title: "[Typings Inconsistency] "
labels: [inconsistency]
assignees: []
description: "Report inconsistencies between TypeScript's DOM typings and browser behavior."
Copy link
Contributor

Choose a reason for hiding this comment

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

I keep being annoyed by usage of DOM in TypeScript, it's really a legacy name that nobody understands. Would be nicer if we call it "web API" here.

options:
- label: "This API is supported in at least two major browsers."
validations:
required: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to add "already tried @types/web package" and "already tried the latest TypeScript release", given people sometimes file issues based on outdated libs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing mentions @types/web though?

@Bashamega Bashamega requested review from saschanaz March 31, 2025 04:37
Co-authored-by: Holger Jeromin <mailgithub@katur.de>
@Bashamega Bashamega requested a review from HolgerJeromin March 31, 2025 06:56
options:
- label: "This API is supported in at least two major browsers."
validations:
required: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing mentions @types/web though?

@@ -0,0 +1,6 @@
blank_issues_enabled: true
issue_templates:
- name: 🔍 Typings Inconsistency
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, I forgot to submit this comment, sorry:

Type definition is what DefinitelyTyped calls it, so let's use that word. Maybe "Web API type definition issue". An issue would be more general than inconsistency which wouldn't cover type enhancements.

@Bashamega
Copy link
Contributor Author

Done @saschanaz

Co-authored-by: Kagami Sascha Rosylight <saschanaz@outlook.com>
@@ -55,7 +55,7 @@ body:
id: use_latest_typescript_web
attributes:
label: "Use the latest @types/web Package"
Copy link
Contributor

Choose a reason for hiding this comment

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

No "use" but "have tried". Same for the label, options, and the TS version above.

@Bashamega
Copy link
Contributor Author

I have fixed it with some issues that I have found @saschanaz

Co-authored-by: Kagami Sascha Rosylight <saschanaz@outlook.com>
@saschanaz
Copy link
Contributor

Looks good to me, this requires additional review from Microsoft.

description: "Paste the TypeScript playground link"
placeholder: "https://www.typescriptlang.org/play/..."
validations:
required: true
Copy link
Member

Choose a reason for hiding this comment

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

Are these elements required on purpose? Is the intent that anything outside this specific format should be a blank issue? I can imagine someone reporting a bug without a playground link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is my intent because we need a playground link to help us reproduce the issue. If you think it shouldn't be required I can do that :)

Copy link
Member

Choose a reason for hiding this comment

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

If @saschanaz is happy, then I'm happy; I'm just double checking as I haven't read everything that happened over the course of the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with making it not required, sometimes it's about conflict with third party type definitions and that wouldn't be reproducible with playground.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made it optional @jakebailey

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

Okay, let's give it a go, then.

@jakebailey jakebailey merged commit 1a786e4 into microsoft:main Apr 15, 2025
6 checks passed
@jakebailey
Copy link
Member

@saschanaz
Copy link
Contributor

That looks fine to me!

@saschanaz
Copy link
Contributor

And thanks @Bashamega, hopefully less issues will reference older lib versions now 🙏🏻

@Bashamega
Copy link
Contributor Author

And thanks @Bashamega, hopefully less issues will reference older lib versions now 🙏🏻

You are welcome

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

Successfully merging this pull request may close these issues.

Add an issue template
5 participants