chore(monorepo): initial commit to create the create-pkg monorepo #18
Conversation
rxmarbles
requested review from
darcyclarke,
ljharb,
mcollina,
mhdawson,
ruyadorno and
wesleytodd
There was a problem hiding this comment.
That was fast! Thanks for the quick work, I made a few comments but they are really just nitpicks. Just having this in place makes adding the other packages so much easier. In your slack DM you mentioned using lerna import but the git commands are pretty easy to pull in a subtree (not easy enough to remember them off hand, but easy enough that I can look them up). But either way we can deal with that when we pull in the first of the other packages.
.npmrc
Outdated
| @@ -1 +1 @@ | |||
| package-lock=false | |||
| registry=https://registry.npmjs.org | |||
There was a problem hiding this comment.
I would rather drop this since it is default personally. Not super opinionated about it, but we can delete the file and who doesn't like deleting files?
There was a problem hiding this comment.
I mostly put this here incase those who have private regsitries and use their work machine instead of their own personal machine. This just makes sure we are absolutely pointed to the npmjs registry. If we feel we don't need it I can remove it.
There was a problem hiding this comment.
Your reasoning makes sense, but home directory .npmrc files are an anti-pattern and anyone using a private registry should be strongly suggesting that projects have their own .npmrc file which points at their internal registry. Any other configuration is a security risk and causes more friction when on-boarding new engineers.
So yes, personally I would rather not make it easier for folks using home directory configs by adding this override.
There was a problem hiding this comment.
Notably, the source of packages shouldn't matter so long as we are generating lockfiles (which is what this change aims to do). Integrity values shouldn't change across the registry mirrors/caches & would get caught by any kind of security-specific tooling.
ie. I'm a +1 to remove this file all together if we're leaning on the defaults
There was a problem hiding this comment.
Notably, the source of packages shouldn't matter so long as we are generating lockfiles
I think I disagree on this point. I think an organization who runs their own registry typically wants to ensure that ALL packages come from that registry. Even if it is the same content with the same integrity values, having server logs for installs is an important factory for incident response. I know for sure I don't want any packages coming from anywhere other than our registry at Netflix.
packages/create-pkg-cli/package.json
Outdated
| "standard": "^14.3.1", | ||
| "standard-version": "^9.0.0" | ||
| }, | ||
| "support": true |
There was a problem hiding this comment.
Was this added in this PR? Not sure it matters, but I just didn't remember it being there in the old one. Also, should be we a bit more particular about our support and use a bit more of the spec?
There was a problem hiding this comment.
Yeah not sure how that slipped in. It might have been in there already but wasn't known until i moved stuff around. We can definitely add more to the support spec for sure.
There was a problem hiding this comment.
Doesn't need to block this PR, especially since support for that field is basically non-existent lol. Just something I noticed.
| package-ecosystem: npm | ||
| rebase-strategy: auto | ||
| schedule: | ||
| interval: daily |
There was a problem hiding this comment.
I got some feedback from folks on another repo in here that we have been using renovate because dependabot is such trash. I would rather add something than nothing but also just wanted to call it out since this was new for this repo.
There was a problem hiding this comment.
Fine with either tbh. We can remove this once we get the renovate setup in the repo
There was a problem hiding this comment.
Maybe @ljharb or @dominykas want to chime in? I feel like it was their feedback but my memory might be off.
|
I got curious, so I looked up |
|
Lerna isn't necessary afaik to retain git history. Ive done migrations like this with git alone. But if lerna worked then no big deal let's ship it 🚀 |
|
Yeah, maybe it is not worth even worrying about this until the first package is pulled in? We only need to pull in like 2 things afaik so we should't get too hung up on it. |
…asted due to issue in npm test cycle
|
We were on the call and @darcyclarke and I both just said we are good to merge. So I just merged. |
This PR initalizes the move of create-pkg from a singular repo to a monorepo starting w/ the create-pkg CLI as per #16 . There are things that are needed to be added at the root README level which I'm hoping to structure out once we move more packages in. We can also link to the package-meta-interop collabspace for more of the research and drivers for the work being done to these packages
cc @Ethan-Arrowood
In regards to maintaining git history for the rest of the packages that we plan on moving into the repo we can use
lerna import. Once we have all the packages that we feel are necessary for the project we can freely remove lerna and go with a purenpm workspacessetup. Just my 2 cents but feel it would get us going on the project and implement/experiment quickly to work towards our goals. Happy to discuss further.