-
-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat: add report an issue button #616
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for pt-br-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for new-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ja-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for zh-hans-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for de-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for es-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for fr-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for hi-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Is it possible to also include the configuration information in the issue? |
The playground link will have the configuration. Should we add the configuration inside |
+1 We should be able to get the necessary configuration information from the playground link, which should be sufficient I guess. If you'd like, we can also include the configuration details explicitly. We already have the configFileContent that we use for downloading the configuration, so we can use the same approach for this. |
Yes, I think it would be nice to output the configuration into the issue as well. It just makes it easier when reading the issue. |
To prefill the configuration to description box need to merge this eslint/eslint#18817. This was a miss as last time I thought we wouldn't want to prefill description. Should we add ids to all elements? |
eslint/eslint#18817 was merged. |
We will now add configuration file's content to report |
Co-authored-by: Amaresh S M <[email protected]>
Unfortunately, the configuration really isn't readable like this if there's more than a couple of rules. Is it not possible to add the three backticks before and after, or otherwise to format it a bit? Can we include the code, too? And can we include the playground link to jump right back to it? |
Should we disable the button with some error message when the users add many rules (i.e. more than 53 rules)? |
It might be nice to just check the length of the URL and show an error message in the playground when they click Share URL |
it also depends on how much code the user has written so we can't rely only on the number of rules the user has selected. Im not sure about the length that github supports. I can see that TSESLint playground also has the same issue where if we write way too much code we get the exact same error in github. |
Yes, we can't display an error message based on the number of rules, but we can validate the length of the URL and ensure it doesn't exceed a certain limit. |
That seems like a good idea. Just pop up an error if the URL gets too long. I think that's a better experience than letting the browser error out. |
The total supported length of the search param seems to be |
fixed it |
@eslint/eslint-tsc can we close this please? |
Prerequisites checklist
What is the purpose of this pull request?
What changes did you make? (Give an overview)
eslint
repoRelated Issues
fixes #598
Is there anything you'd like reviewers to focus on?
should we do it for any other field?