Skip to content

docs: use valid rule config in the custom rule example #4760

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 30, 2022
Merged

docs: use valid rule config in the custom rule example #4760

merged 5 commits into from
Mar 30, 2022

Conversation

alceil
Copy link
Contributor

@alceil alceil commented Mar 29, 2022 •
edited
Loading

PR Checklist

@nx-cloud
Copy link

nx-cloud bot commented Mar 29, 2022 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 209be3c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 43 targets

Sent with 💌 from NxCloud.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @alceil!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@netlify
Copy link

netlify bot commented Mar 29, 2022 •
edited
Loading

✅ Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 209be3c
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/6244d3ae4209100009b70bef
😎 Deploy Preview https://deploy-preview-4760--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 29, 2022
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

A good start, thanks for sending this!

Try copy & pasting the code into the TypeScript playground and you'll see it's still missing two properties:

  • defaultOptions
  • name

@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Mar 29, 2022
@alceil
Copy link
Contributor Author

alceil commented Mar 29, 2022

Got it lemme try @JoshuaKGoldberg

@alceil
Copy link
Contributor Author

alceil commented Mar 29, 2022

I have done the required changes please have a look @JoshuaKGoldberg

JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 29, 2022
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Closer! 🙂

@JoshuaKGoldberg
Copy link
Member

Btw, you don't need to @ tag me to get a review - we get notifications for this repo's events. Thanks though!

@bradzacher bradzacher changed the title Docs: Use valid rule config in the custom rule example docs: use valid rule config in the custom rule example Mar 29, 2022
@alceil
Copy link
Contributor Author

alceil commented Mar 30, 2022

Sure

@alceil
Copy link
Contributor Author

alceil commented Mar 30, 2022

I have done the requested changes.

@alceil
Copy link
Contributor Author

alceil commented Mar 30, 2022 •
edited
Loading

I have a doubt @JoshuaKGoldberg prettier works only on ts or other extension files right. It doesn't work on markdown files If I'm right. So I just ran prettier to the code snippet I made changes on. Is it alright?

@JoshuaKGoldberg
Copy link
Member

work on markdown files

Prettier does work on Markdown files. It's listed on https://prettier.io and https://prettier.io/docs/en/index.html. Prettier will format code inside Markdown code blocks in the language of the code block.

We can see in the PR checks that there is still a formatting issue with the code:

Screenshot of failing PR check: Code style and lint

If you click the Details link there you'll be taken to a page that shows the error log: https://github.com/typescript-eslint/typescript-eslint/runs/5748785507?check_suite_focus=true

$ prettier --list-different "./**/*.{md,mdx,ts,js,tsx,jsx}"
docs/development/CUSTOM_RULES.md

CI is running the prettier command on all md, mdx, and a few other file extensions. The only failing file is docs/development/CUSTOM_RULES.md.

If this is new and inconvenient for you, let me know and I can run Prettier on it for you 🙂. We just don't by default as a lot of folks use pull requests as an opportunity to learn tooling such as Prettier.

If you do want to do this yourself (yay!), click this for a more explicit list of steps than my comment above to edit in an online editor.
  1. Visit https://github.dev/alceil/typescript-eslint/tree/patch-1
  2. In the bottom-right corner, it should ask you if you want to install recommended extensions. Press Install.
    VS Code popup asking to install recommended extensions
  3. If anything gives you a warning about limited functionality, press Install anyway.
    VS Code popup asking to install markdownlint despite limited functionality
  4. Click the file explorer tab on the left, then find and open docs/development/CUSTOM_RULES.md
    File explorer open to docs/development/CUSTOM_RULES.md
    • Alternately, use the file selector (Cmd+P on Mac, or Ctrl+P on Windows) to search for that file and Enter to open it
  5. Save the file (shortcut: Cmd+S on Mac, or Ctrl+S on Windows)
  6. Click the source control tab on the left, then in the text input, type any message vaguely like ran prettier
    Source control with the message 'ran prettier'
  7. Click the check icon (label: Commit) to make a commit, then the refresh icon just to its right (label: Refresh) to push that commit up to GitHub
    Source control hovering over the Commit icon

JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 30, 2022
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

I think you ran Prettier with a different set of settings than this repository uses. 🙂

@alceil
Copy link
Contributor Author

alceil commented Mar 30, 2022

I ran the command mentioned in your recent comments but still, It has no changes to the markdown file. I researched on the internet about it and it seems many have been facing this issue. So can you run prettier for me @JoshuaKGoldberg

@JoshuaKGoldberg
Copy link
Member

No worries, thanks for sending this & working with me on it! 💖

@JoshuaKGoldberg JoshuaKGoldberg removed the awaiting response Issues waiting for a reply from the OP or another party label Mar 30, 2022
@JoshuaKGoldberg JoshuaKGoldberg merged commit 8c98d16 into typescript-eslint:main Mar 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs: Use valid rule config in the custom rule example
2 participants
@alceil @JoshuaKGoldberg