Skip to content

chore: setup Yarn constraints#13363

Merged
nicolo-ribaudo merged 11 commits intobabel:mainfrom
merceyz:merceyz/constraints
May 31, 2021
Merged

chore: setup Yarn constraints#13363
nicolo-ribaudo merged 11 commits intobabel:mainfrom
merceyz:merceyz/constraints

Conversation

@merceyz
Copy link
Copy Markdown
Contributor

@merceyz merceyz commented May 24, 2021

Q                       A
Tests Added + Pass? CI check added
Any Dependency Changes? Yarn
License MIT

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.

@merceyz merceyz force-pushed the merceyz/constraints branch from 5543612 to 185fcaa Compare May 24, 2021 20:11
@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented May 24, 2021

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:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented May 24, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/46566/

Comment thread constraints.pro
Comment on lines +55 to +56
gen_enforced_field(WorkspaceCwd, 'author', 'The Babel Team (https://babel.dev/team)') :-
\+ workspace_field(WorkspaceCwd, 'private', true).
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo May 24, 2021

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.

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

@julienw julienw Jun 11, 2021

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 :-)

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label May 24, 2021
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a 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 "./?

Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread constraints.pro
Comment on lines +34 to +37
% 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',
Copy link
Copy Markdown
Contributor Author

@merceyz merceyz May 25, 2021

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?

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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?

Copy link
Copy Markdown
Member

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.

@merceyz merceyz force-pushed the merceyz/constraints branch 3 times, most recently from 889f988 to a685df9 Compare May 26, 2021 16:42
@merceyz merceyz force-pushed the merceyz/constraints branch from a685df9 to 9ed9ebe Compare May 27, 2021 15:51
@github-actions github-actions Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Sep 11, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants