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

Enhance config discovery with cosmiconfig #53911

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

MericKarabulut
Copy link
Contributor

@MericKarabulut MericKarabulut commented Aug 24, 2023

What?

The modifications made here align with the objectives set out in issue #30842.

Why?

in @wordpress/scripts package lint-js, lint-pkg-json and lint-style scripts try's to locate relevant configs using hasProjectFile repeatedly.

How?

By utilizing cosmiconfig, a tool crafted to establish a standardized approach for identifying configuration files across different tools, we can simplify the logic of configuration file discovery. Moreover, this approach allows us to enhance performance by minimizing the redundant invocation of helper functions.

Testing Instructions

  1. install the necessary dependencies.
  2. Set up a test configuration file for any of the affected scripts (e.g., lint-js, lint-pkg-json, or lint-style).
  3. Run the respective script and verify that the configuration is correctly detected and utilized.
  4. Ensure that existing functionality remains unaffected.

Testing Instructions for Keyboard

none

Screenshots or screencast

none

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Aug 24, 2023
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @MericKarabulut! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@gziolo gziolo added [Tool] WP Scripts /packages/scripts [Type] Code Quality Issues or PRs that relate to code quality labels Aug 24, 2023
@MericKarabulut
Copy link
Contributor Author

Hey there 👋,

I've been working on the code and it seemed to be running smoothly. However, the tests, they unexpectedly failed. I've gone through the code and checked configurations, but I'm hitting a bit of a roadblock in identifying the exact cause. If someone could take a look and provide some insights, that would be fantastic! 🙏

@MericKarabulut MericKarabulut force-pushed the try/cosmiconfig-integration branch from 1f8cc33 to dc363be Compare August 26, 2023 15:44
@gziolo gziolo requested a review from ockham August 30, 2023 09:13
@gziolo
Copy link
Member

gziolo commented Sep 1, 2023

Thank you so much for starting this PR.

There are some issues reported by Continues Integration:

image

There might be a subtle issue with how the paths are checked

@gziolo
Copy link
Member

gziolo commented Sep 2, 2023

I think I know why some checks are red. It turns out that cosmiconf doesn't work with all file extensions by default. They don't plan to support .config.json as explained in cosmiconfig/cosmiconfig#246 (comment). It can be supported with their API:

Please consider using searchPlaces to implement this feature.

However, we can always combine some existing checks with cosmiconf if that is simpler to maintain.

Edit: It doesn't look like .config.json was listed before, so it is probably something else.

@gziolo gziolo added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Tool] WP Scripts /packages/scripts [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants