search

matrix-org / matrix-rust-sdk-crypto-nodejs

Apache License 2.0
3 stars 5 forks source link

Post-repo-split stuff #1

Closed jplatte closed 1 year ago

jplatte commented 1 year ago

Everything before this PR in this repo is simply the result of a git subtree split, i.e. it has exactly the same history as the bindings/matrix-sdk-crypto-nodejs directory in matrix-rust-sdk.

jplatte commented 1 year ago

So, with commented-out if's for pages deployment, docs seem to be working fine. I think this can be merged, and afterwards https://github.com/matrix-org/matrix-rust-sdk/pull/2264 can. The only thing I'm not sure how to test is the release stuff.

AndrewFerr commented 1 year ago

When I attempt to build this locally, it hits this failure:

~/git/matrix-rust-sdk-crypto-nodejs (main)$ npm install --ignore-scripts && npm run build -- --features tracing

added 295 packages, and audited 296 packages in 31s

35 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> @matrix-org/matrix-sdk-crypto-nodejs@0.1.0-beta.1 build
> napi build --platform --features tracing

Type Error: Could not parse the Cargo.toml: Error: Command failed: cargo metadata --format-version 1 --manifest-path "/home/andrewf/git/matrix-rust-sdk-crypto-nodejs/Cargo.toml"
error: failed to parse manifest at `/home/andrewf/git/matrix-rust-sdk-crypto-nodejs/Cargo.toml`

Caused by:
  error inheriting `rust_version` from workspace root manifest's `workspace.package.rust_version`

Caused by:
  failed to find a workspace root

error: failed to parse manifest at `/home/andrewf/git/matrix-rust-sdk-crypto-nodejs/Cargo.toml`

Caused by:
  error inheriting `rust_version` from workspace root manifest's `workspace.package.rust_version`

Caused by:
  failed to find a workspace root

<snip: traceback>

Pardon the naive question, but do I have to clear some kind of workspace-related setting that was applied by previous (successful) builds of the bindings before the repo move?

jplatte commented 1 year ago

Sounds like you are testing main, i.e. the state before this PR?

AndrewFerr commented 1 year ago

D'oh, yes, that was it. It builds just fine now.

One last question: are all of the beta releases (like matrix-sdk-crypto-nodejs-v0.1.0-beta.X) omitted from this repo's git history? In the original repo, they were given branches that never got merged back into main (other than beta.1), which may have prevented them from being seen by the git subtree split.

This isn't too urgent, but asking since I typically test with tagged releases & wanted to know if that practice would/should change (even if just for the short term until new releases are cut).

jplatte commented 1 year ago

In the original repo, they were given branches that never got merged back into main

Huh, why is that? I'm sure there is a way to keep those, but I wonder if it makes sense. The tags and branches will continue to exist on the other repo anyways (unless we decide to delete them).

AndrewFerr commented 1 year ago

Not sure why the Node bindings releases were done that way, especially since the JS & FFI bindings releases appear in main.

This also caused problems with the Node bindings' release workflow, where every new release would think the previous release was beta.1 (since it was the only release in main).

I'd be fine with losing the history of previous releases, as long as there can be a new release cut from this repo sometime soon, to serve as an example of a clean release to use.