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.

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

Add gts/gjs configs #1944

Merged
merged 1 commit into from Nov 2, 2023
Merged

Add gts/gjs configs #1944

merged 1 commit into from Nov 2, 2023

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Aug 24, 2023 •
edited

@patricklx patricklx force-pushed the gts/gjs-configs branch 4 times, most recently from 3bca299 to 7c6ffc8 Compare August 24, 2023 12:47
fry69
fry69 reviewed Aug 26, 2023
View reviewed changes
README.md Outdated
|:------------------------------------|:--------------------------------------------------|:------------------------------------|
| ✅ | [recommended](./lib/recommended-rules.js) | enables the `recommended` rules |
| ![badge-gjs](docs%2Fsvgs%2Fgjs.svg) | [recommended-gjs](./lib/recommended-rules-gjs.js) | enables the `recommended-gjs` rules |
| ![badge-gjs](docs%2Fsvgs%2Fgts.svg) | [recommended-gts](./lib/recommended-rules-gjs.js) | enables the `recommended-gjs` rules |
Copy link

Choose a reason for hiding this comment

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

I think the gjs here in the link and in the text in this line should be gts.

fry69
fry69 reviewed Aug 26, 2023
View reviewed changes
README.md Outdated
@@ -238,3 +280,6 @@ Note that new rules should not immediately be added to the [recommended](./lib/r
## 🔓 License

See the [LICENSE](LICENSE.md) file for license rights and limitations (MIT).

[badge-recommended-gjs]: docs%2Fsvgs%2Fgjs.svg
[badge-recommended-gts]: docs%2Fsvgs%2Fgjs.svg
Copy link

@fry69 fry69 Aug 26, 2023 •
edited

Choose a reason for hiding this comment

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

This should point to gts.svg, not gjs.svg

@patricklx patricklx mentioned this pull request Aug 29, 2023
Merged
@patricklx patricklx marked this pull request as ready for review August 29, 2023 07:21
fry69
fry69 reviewed Aug 29, 2023
View reviewed changes
README.md Outdated
|:------------------------------------|:--------------------------------------------------|:------------------------------------|
| ✅ | [recommended](./lib/recommended-rules.js) | enables the `recommended` rules |
| ![badge-gjs](docs%2Fsvgs%2Fgjs.svg) | [recommended-gjs](./lib/recommended-rules-gjs.js) | enables the `recommended-gjs` rules |
| ![badge-gjs](docs%2Fsvgs%2Fgts.svg) | [recommended-gts](./lib/recommended-rules-gts.js) | enables the `recommended-gts` rules |
Copy link

Choose a reason for hiding this comment

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

![badge-gjs] -> ![badge-gts]

@patricklx patricklx force-pushed the gts/gjs-configs branch 3 times, most recently from 36cc77b to 26328a9 Compare August 29, 2023 07:48
@patricklx patricklx mentioned this pull request Oct 31, 2023
Closed
bmish
bmish reviewed Oct 31, 2023
View reviewed changes
lib/config/base.js
@@ -0,0 +1,37 @@
module.exports = {
Copy link
Member

@bmish bmish Oct 31, 2023 •
edited

Choose a reason for hiding this comment

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

FYI we had a base config before. I removed it when it didn't seem necessary #1518. It's fine if you want to add it back. We had some other interest in adding back the base config here too: #1895

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, i think it makes sense if want to go this way and provide recommended configs per file type.
btw, eslint-typescipt also has a base config (but I didnt see it documented) https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/base.ts

Copy link
Member

Choose a reason for hiding this comment

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

Yep, seems fine to add base.

@patricklx patricklx force-pushed the gts/gjs-configs branch 8 times, most recently from ae34d4b to 3370a16 Compare November 2, 2023 10:38
@patricklx
Copy link
Contributor Author

patricklx commented Nov 2, 2023 •
edited

@NullVoxPopuli updated the eslintrc example

@NullVoxPopuli NullVoxPopuli merged commit e42d472 into ember-cli:master Nov 2, 2023
8 checks passed
@NullVoxPopuli NullVoxPopuli changed the title gts/gjs configs add gts/gjs configs Nov 2, 2023
@patricklx patricklx deleted the gts/gjs-configs branch November 2, 2023 14:04
bmish
bmish reviewed Nov 2, 2023
View reviewed changes
README.md
| | Name |
| :- | :------------ |
| ✅ | `recommended` |
| | Name |
Copy link
Member

Choose a reason for hiding this comment

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

If you add a description property to each config, it will show up in this table. Up to you if you think it would be helpful.

@bmish bmish changed the title add gts/gjs configs Add gts/gjs configs Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmish bmish bmish left review comments

@fry69 fry69 fry69 left review comments

@NullVoxPopuli NullVoxPopuli NullVoxPopuli approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@patricklx @NullVoxPopuli @bmish @fry69