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

Contribution guide/setup #15

Closed
43081j opened this issue Dec 26, 2024 · 5 comments
Closed

Contribution guide/setup #15

43081j opened this issue Dec 26, 2024 · 5 comments

Comments

@43081j
Copy link
Contributor

43081j commented Dec 26, 2024

Could you let us know what your workflow is when developing in this repo?

Maybe we can then document that in the README or a CONTRIBUTING doc

Locally, I've tried the following steps to build and test the client:

  • corepack enable && pnpm i
  • fails on node 23.x for me because of better-sqlite3's make failing with "C++20 or later required"
  • so I then thought let's try an older node, 20 will do. install now succeeds
  • next, there's no root build script afaict, so I figure out the dependencies and individually build each one so client can ultimately build
  • at this point, I try pnpm test in the client dir. this fails because better-sqlite3 wants NODE_MODULE_VERSION 131 and I have a lower one
  • it turns out 131 appears to be 23.x, which I can't use since the install build/make fails (as seen above, chicken and egg situation)

so I think we just need to know what workflow you've been using to not run into any of this

node version, package manager, how you run builds, etc etc

i'd be happy to write the doc up once I understand, and maybe a root build and test script

edit:

with node 20.x I managed to get it all working 🙌

@mary-ext
Copy link
Owner

mary-ext commented Dec 26, 2024

would this added contribution guide section help?
d161527

@43081j
Copy link
Contributor Author

43081j commented Dec 26, 2024

Awesome, that looks good to me

It may also be worth mentioning you can run tests for one package using pnpm run --filter="@atproto/client"

Everything does seem to work using node 20 in the end

My setup was using corepack:

nvm use 20
corepack enable
pnpm i

@mary-ext
Copy link
Owner

sweet, I'll tack in a mention for pnpm as the package manager of choice, and a way to run tests for a specific package (normally when I work on a specific package, I'd just have a shell with that package's folder open)

@43081j
Copy link
Contributor Author

43081j commented Dec 26, 2024

same here re having a shell open in the sub-package's directory

i think as long as we mention one of them (cd into the dir, or use filter), it makes it clear what the workflow is

thanks for looking at this so quickly too

@43081j
Copy link
Contributor Author

43081j commented Dec 26, 2024

Closing since we now have a guide in the readme 👍

@43081j 43081j closed this as completed Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants