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

chore(cli): change helper folder structure #240

Merged
merged 7 commits into from
Aug 4, 2023

Conversation

eunjae-lee
Copy link
Contributor

What?

This PR changes the folder structure for the helpers as discussed in #224

Why?

JIRA: EXT-1887

How to test? (optional)

@vercel
Copy link

vercel bot commented Aug 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plugin-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 1, 2023 2:58pm

@eunjae-lee eunjae-lee force-pushed the EXT-1887-change-folder-structure-regarding-helpers branch from a22c015 to 8db177e Compare August 1, 2023 14:24
Comment on lines +27 to +28
yarn eslint packages/field-plugin --ignore-pattern "packages/field-plugin/helpers/"
yarn workspaces foreach --include "helper-*" run lint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • line 27: ignore helpers from eslint
  • line 28: run eslint for helpers with their own configs

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should they run separately?

Does this not solve the problem:

helper-vue3/.eslintrc.json

{
  "root": true,
  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't work. As far as I understand, the moment you run yarn eslint packages/field-plugin, it loads the root eslint config and apply it to all the files below. That root: true prevents your eslint process from "going up". So I had to separate them to make them work. Maybe I should just ignore the helpers in the root eslint config, so that I don't have to explicitly exclude them in the command line?

@eunjae-lee eunjae-lee marked this pull request as ready for review August 1, 2023 14:58
@eunjae-lee eunjae-lee requested review from a team and johannes-lindgren and removed request for a team August 1, 2023 14:58
@eunjae-lee
Copy link
Contributor Author

@BibiSebi this has changed since you worked on it :)

Copy link
Collaborator

@johannes-lindgren johannes-lindgren left a comment

Choose a reason for hiding this comment

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

Pretty straightforward; I just wonder about the eslint config

Comment on lines +27 to +28
yarn eslint packages/field-plugin --ignore-pattern "packages/field-plugin/helpers/"
yarn workspaces foreach --include "helper-*" run lint
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should they run separately?

Does this not solve the problem:

helper-vue3/.eslintrc.json

{
  "root": true,
  ...
}

@eunjae-lee eunjae-lee merged commit 2c7d23c into main Aug 4, 2023
@eunjae-lee eunjae-lee deleted the EXT-1887-change-folder-structure-regarding-helpers branch August 4, 2023 13:18
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.

2 participants