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

feat: support flat config #161

Merged
merged 24 commits into from
Mar 24, 2024

Conversation

hyoban
Copy link
Contributor

@hyoban hyoban commented Feb 20, 2024

PR Checklist

Overview

Some code are not very good, looking forward to your suggestions

@hyoban hyoban marked this pull request as draft February 20, 2024 15:53
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: Patch coverage is 98.48485% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 99.22%. Comparing base (8d9ed75) to head (591f08f).
Report is 94 commits behind head on main.

Files Patch % Lines
src/plugin.ts 97.72% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #161      +/-   ##
==========================================
- Coverage   99.30%   99.22%   -0.08%     
==========================================
  Files          15       17       +2     
  Lines         866      907      +41     
  Branches       73       74       +1     
==========================================
+ Hits          860      900      +40     
- Misses          6        7       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hyoban hyoban marked this pull request as ready for review February 20, 2024 16:32
Copy link
Owner

@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.

Ok! So sorry for taking multiple weeks to get back to you on this - it's a good change and important for the plugin. Thank you very much for getting it started!

The general path you took here seems reasonable to me. Glad to see the actual changes to src/index.ts are pretty straightforward. I left some requests inline about streamlining how the dual-emit (CJS/ESM) and dual-format (flat/legacy) support should look.

Also, per #91 (comment), requesting:

Document how to use them with flat config in the README.md

I think I'd made a note to get back to this PR once that was in and forgot to follow up 😅.

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@JoshuaKGoldberg JoshuaKGoldberg added the status: waiting for author Needs an action taken by the original poster label Mar 22, 2024
@hyoban
Copy link
Contributor Author

hyoban commented Mar 24, 2024

Hi, I have changed the code to apply your suggestion. I intentionally left the documentation section until the implementation is refined based on your review, which has now been added.

Thanks a lot for your review. I have learned a lot.

@github-actions github-actions bot removed the status: waiting for author Needs an action taken by the original poster label Mar 24, 2024
Copy link
Owner

@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.

This looks great, thanks a bunch @hyoban! 🙌

I only have some small, honestly non-blocking comments left. Do you have opinions on them?

If not, no worries, I've set a reminder to merge this on Wednesday. 🚀

.eslintrc.cjs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/plugin.ts Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@hyoban
Copy link
Contributor Author

hyoban commented Mar 24, 2024

@JoshuaKGoldberg I have no problem with your modification, but I don't have a computer with me now. You can modify and merge it directly. Or I will deal with it tomorrow.

@JoshuaKGoldberg
Copy link
Owner

You got it, I'll go ahead and do that now so we can release today. Thanks!

Copy link
Owner

Choose a reason for hiding this comment

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

Figured I'd add a quick one here so code coverage doesn't drop. 🤷

@JoshuaKGoldberg JoshuaKGoldberg merged commit bf31a6c into JoshuaKGoldberg:main Mar 24, 2024
12 of 14 checks passed
Copy link

🎉 This is included in version v0.11.0 🎉

The release is available on:

Cheers! 📦🚀

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.

🚀 Feature: Support flat config
2 participants