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

build: Update to lerna 7.x #1417

Merged
merged 4 commits into from
Jul 24, 2023
Merged

build: Update to lerna 7.x #1417

merged 4 commits into from
Jul 24, 2023

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jul 21, 2023

- Removed `useWorkspaces` flag (no longer used)
- Removed 'useNx' flag (NX seems to work in the latest)
- Add provenance flag to .npmrc
- Fixes deephaven#1235
- Tested `npm start`, `npm test`, and `npm run build` steps and ensured they all worked correctly
@mofojed mofojed requested a review from mattrunyon July 21, 2023 16:03
@mofojed mofojed self-assigned this Jul 21, 2023
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #1417 (22ad468) into main (7480280) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1417   +/-   ##
=======================================
  Coverage   45.71%   45.71%           
=======================================
  Files         511      511           
  Lines       35065    35065           
  Branches     8767     8767           
=======================================
  Hits        16029    16029           
  Misses      18985    18985           
  Partials       51       51           
Flag Coverage Δ
unit 45.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

lerna.json Outdated Show resolved Hide resolved
- lerna v7 now uses the npm workspaces config by default
- Remove the `packages` field in lerna.json which was overriding this and no longer necessary
@mattrunyon
Copy link
Collaborator

Have you tried publishing an alpha version from this branch? Should probably do that to ensure the provenance stuff works

@mattrunyon
Copy link
Collaborator

There is also an issue w/ caching and build. I ran a build, changed vite.config.ts, then ran build again and it used the cached build. Probably something I didn't get right when I tried setting up nx the first time.

And another issue w/ nx is it causes .env files to load differently and doesn't play nicely with Vite. npm start doesn't load .env.development b/c nx loads .env first, so Vite does not override previously set env variables. The LogProxy is on in dev mode (should be off) and the base URL is relative and not /ide/ like .env.development specifies

See #1011 (comment) for previous issues

@mofojed
Copy link
Member Author

mofojed commented Jul 24, 2023

I'll just add back the useNx: false flag - that wasn't the point of this change anyways, leave debugging that issue to another time. Good catch though!

- There were still some issues with using nx: deephaven#1011 (comment)
@mattrunyon
Copy link
Collaborator

Need to add provenance=true to .nvmrc or "publishConfig": { "provenance": true } to package.json to enable it

- Followed the instructions on https://docs.npmjs.com/generating-provenance-statements#publishing-packages-with-provenance-via-github-actions
- Not sure why I pushed this without setting this flag at all
Copy link
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Looks like the action change will need to merge before we can validate this publishes properly, but looks like it should based on the failed publish saying it needs the id-token permission

@mofojed mofojed merged commit 29a7d8d into deephaven:main Jul 24, 2023
@mofojed mofojed deleted the 1235-provenance branch July 24, 2023 21:03
@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add provenance to published npm packages
2 participants