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: initialize monorepo setup #612

Merged
merged 9 commits into from
Jul 15, 2024
Merged

chore: initialize monorepo setup #612

merged 9 commits into from
Jul 15, 2024

Conversation

snitin315
Copy link
Contributor

@snitin315 snitin315 commented Jul 3, 2024

Refers #609

Changes

  • Move all Espree-specific files into packages/espree
  • Update release-please configuration to be a monorepo setup (like `eslint/rewrite)
  • Update root README so it's not Espree-specific

Additional information

  • This PR provides initial monorepo setup & moves espree to packages/espree. I'll create separate PRs to move rest of the packages to this repo. There are already too many files changed that the GitHub UI lags when going through file changes. Or do we want to move all the packages in one PR?
  • Not publishing espree to JSR, like other packages in the rewrite repo.

@snitin315 snitin315 marked this pull request as ready for review July 6, 2024 11:59
Co-authored-by: Nicholas C. Zakas <[email protected]>
@snitin315 snitin315 requested a review from nzakas July 6, 2024 12:06
nzakas
nzakas previously approved these changes Jul 8, 2024
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Would like @mdjermanovic and @fasttime to review before merging.

packages/espree/package.json Outdated Show resolved Hide resolved
packages/espree/package.json Outdated Show resolved Hide resolved
eslint.config.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
.github/workflows/release-please.yml Outdated Show resolved Hide resolved
.github/workflows/release-please.yml Outdated Show resolved Hide resolved
.github/workflows/release-please.yml Outdated Show resolved Hide resolved
.github/workflows/release-please.yml Outdated Show resolved Hide resolved
mdjermanovic
mdjermanovic previously approved these changes Jul 9, 2024
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Leaving open for @nzakas to verify the latest changes and re-approve, and for @fasttime to review.

nzakas
nzakas previously approved these changes Jul 9, 2024
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Looking for one last review from @fasttime

@fasttime
Copy link
Member

I'm planning on reviewing this tomorrow.

README.md Outdated
Comment on lines 180 to 182
## Security Policy

We work hard to ensure that Espree is safe for everyone and that security issues are addressed quickly and responsibly. Read the full [security policy](https://github.com/eslint/.github/blob/master/SECURITY.md).
Copy link
Member

Choose a reason for hiding this comment

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

The security policy section is required to comply with one of Tidelift's recommendations. See https://tidelift.com/lifter/package/npm/espree/tasks/packages_have_security_policies.

image

We could change the link in Tidelift so it points to packages/espree/docs/README.md instead, but I think it would be better to keep it pointing to a page that's not inside a package. Opinions?

Copy link
Member

Choose a reason for hiding this comment

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

We should probably have it in both places: one that is Espree-specific in the Espree package and one at the repo root that is more generic.

Copy link
Member

Choose a reason for hiding this comment

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

We should probably have it in both places: one that is Espree-specific in the Espree package and one at the repo root that is more generic.

Sounds good. We could re-add the security policy to the main README.md file but replace "Espree" with something more generic (e.g. "packages in this repository"). I think that's the only change needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated root Readme 👍🏻

@snitin315 snitin315 dismissed stale reviews from nzakas and mdjermanovic via 4c225f9 July 14, 2024 01:48
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Would like another review before merging.

Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@fasttime
Copy link
Member

Note: I'm going to update the GitHub repo description after merging, so it's no longer Espree-specific.

image

@fasttime fasttime merged commit 6f4d325 into main Jul 15, 2024
11 checks passed
@fasttime fasttime deleted the monorepo-setup branch July 15, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants