-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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: setup Yarn constraints #13363
Conversation
5543612
to
185fcaa
Compare
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f8d02f1:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/46566/ |
gen_enforced_field(WorkspaceCwd, 'author', 'The Babel Team (https://babel.dev/team)') :- | ||
\+ workspace_field(WorkspaceCwd, 'private', true). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I would love to do it, I'm not 100% sure that we can legally remove Sebastian, Logan and the other people from the existing author
fields. We can do that if git blame
doesn't assign any piece of code to them, but probably it's too much work to check.
Maybe we should add an explicit list of packages that can have a different author (it doesn't need to be maintained anyway, it's "legacy"), and only enforce The Babel Team
for new packages.
We can probably replace @hzoo in @babel/preset-env
if he agrees, since he's still a team member.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I'm not a lawyer the MIT license only requires that the license is included so based on that it's fine to change it as it's part of the "Software" and permission was granted to modify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey!
Not a lawyer here either, but I worked a bit on licenses in the past. I just want to mention that license and author rights are two different things.
I think the change here is fine as long as Sebastian and others' attributions are still present otherwise (an AUTHORS file, or maybe the output git blame
itself is already good enough in this case as long as their contributions are properly labeled in the git repository).
Other free software sometimes uses a CLA that enforces that every contribution's author rights are given to one single organization... but even this doesn't work in some countries (eg: France).
My (unwanted) 2 cents :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a new constraint that all packageJson.main
must start with "./
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
% Exempt from the rule as it supports '>=4'. TODO: remove with the next major | ||
WorkspaceIdent \= '@babel/plugin-proposal-unicode-property-regex', | ||
% Exempt from the rule as it supports '>=6.0.0'. TODO: remove with the next major | ||
WorkspaceIdent \= '@babel/parser', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these two actually built for >=6.9.0
but the engines.node
value wasn't updated back in the day or are they correct and special cased?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good catch, they are both built for 6.9.0
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I let them get updated or wait for the next major to be on the safe side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets keep them for now, just to be safe.
889f988
to
a685df9
Compare
a685df9
to
9ed9ebe
Compare
This PR adds some constraints to enforce consistency across the
package.json
files in the repository, I wrote a few that stood out but if you have any other rules in mind let me know and I can add them. I suggest reviewing each commit on it's own and feel free to revert any you don't want as these are merely suggestions.The checked in version of Yarn and the plugin is built from yarnpkg/berry#2922 to get some fixes that landed in 3.0.0-rc.1 without the breaking changes.