Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: addtypescript-eslint package scaffold#8296

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
bradzacher merged 1 commit intomainfromcore-package-scaffold
Feb 1, 2024

Conversation

bradzacher
Copy link
Member

@bradzacherbradzacher commentedJan 24, 2024
edited
Loading

PR Checklist

Overview

This PR just adds an private scaffold for thetypescript-eslint package being added in#7935. I've split this out so that the automation will bump the versions with each release.

The most annoying part of the branch is every time I go back to it and rebase it onmain I have to manually resolve and correct all of the versions or else things will use old releases from npm instead of the repo code.

@bradzacherbradzacher added the repo maintenancethings to do with maintenance of the repo, and not with code/docs labelJan 24, 2024
@typescript-eslint
Copy link
Contributor

Thanks for the PR,@bradzacher!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint.

@netlifyNetlify
Copy link

netlifybot commentedJan 24, 2024
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit7c3f8cc
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/65bb07b2c91994000866ab56
😎 Deploy Previewhttps://deploy-preview-8296--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 90 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 98 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify site configuration.

@bradzacherbradzacherforce-pushed thecore-package-scaffold branch 2 times, most recently from50df23d to23cc699CompareJanuary 24, 2024 04:32
@merceyz
Copy link

merceyz commentedJan 24, 2024
edited
Loading

I have to manually resolve and correct all of the versions or else things will use old releases from npm instead of the repo code.

You can replace the explicit versions withworkspace:*, Yarn will replace them on publish with the exact version.
https://yarnpkg.com/features/workspaces#cross-references

@bradzacher
Copy link
MemberAuthor

@merceyz oh that's interesting! I didn't know that existed.

I had noticed that there was the syntax when I looked at the lock file (you can see I used it in the root package.json) - but I didn't realise that it would automatically get replaced on publish! That's super cool!

Cc@JamesHenry - we use the nx packages for publishing - does nx useyarn npm publish under the hood or does it not? I.e. Could we use this syntax? Could nx support it?

And for an FYI@JoshuaKGoldberg

JoshuaKGoldberg reacted with thumbs up emoji

@JamesHenry
Copy link
Member

@bradzacher Right now there is a fork in the road when it comes to non semver versions.

Either:

  • you apply your versions using valid semver version in your source files, tracked by git

OR

  • you can leverage either semver versions of any valid workspace/file protocol specifiers if version yourdist files, not tracked by git

This second option is how the Nx repo itself does it. The source of truth for versions in this case is therefore not disk, but rather the npm registry. Nx release supports this case via config in nx.json.


Background

This fundamental choice is down to the implementation ofnx release being based on isolated version/changelog/publish steps, and the fact that we want to publish something tangible on disk.

Some release tools read your files but then perform all kinds of modifications in memory which are not reflected anywhere on your file system for you to see/play with and then publish that in memory object to npm. This can make troubleshooting bad publishes nearly impossible at times - with this approach it's trivial, because you can alwayscd somewhere and runnpm publish directly with whatever options you want.

So when you perform your versioning step,nx release gets your files ready for your publishing step on disk, therefore it needs to replace those workspace/file references with the real numbers so that they work for your consumers.

Now, having said all that...

When we first created this hard line we only had the three subcommands available:nx release version,nx release changelog,nx release publish and therefore was not straightforward way to plumb things through from one to the other.

These days we have the programmatic API fornx release as a first class use-case - it's already what we use in typescript-eslint, and so it is therefore possible for me to imagine exposing a util function on the programmatic API to reset workspaces/file references after runningreleasePublish. The main complication though will be not committing those package.json dependency updates as part of the versioning or changelog step. I think it is doable though.

This is the issue to track for progress:nrwl/nx#20978

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM to start, just requesting changes on the license and missing docs. 🚀

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelJan 24, 2024
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelJan 24, 2024
JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesJan 24, 2024
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

🚢

JamesHenry
JamesHenry previously requested changesJan 24, 2024
Copy link
Member

@JamesHenryJamesHenry left a comment

Choose a reason for hiding this comment

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

Let’s finalize the discord discussion before merging

@bradzacherbradzacher changed the titlechore: addcore package scaffoldchore: addtypescript-eslint package scaffoldJan 31, 2024
@bradzacherbradzacherforce-pushed thecore-package-scaffold branch 2 times, most recently froma9ad49e to7d3db25CompareFebruary 1, 2024 02:46
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Cartoon happy yellow bird bouncing up and down above the word 'YASS'

@bradzacherbradzacher dismissedJamesHenry’sstale reviewFebruary 1, 2024 09:38

Update name as per discussion.

@bradzacherbradzacher merged commit94aa28e intomainFeb 1, 2024
@bradzacherbradzacher deleted the core-package-scaffold branchFebruary 1, 2024 09:45
danvk pushed a commit to danvk/typescript-eslint that referenced this pull requestFeb 4, 2024
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsFeb 9, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

@JamesHenryJamesHenryAwaiting requested review from JamesHenry

Assignees
No one assigned
Labels
repo maintenancethings to do with maintenance of the repo, and not with code/docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@bradzacher@merceyz@JamesHenry@JoshuaKGoldberg

[8]ページ先頭

©2009-2025 Movatter.jp