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

install buf via npm #110

Merged
merged 3 commits into from
Aug 10, 2023
Merged

install buf via npm #110

merged 3 commits into from
Aug 10, 2023

Conversation

sethvincent
Copy link
Contributor

@sethvincent sethvincent commented Aug 9, 2023

closes #105
closes #33

This installs buf from npm and moves the buf command usage out of ci jobs and into a package.json script that's included in build:all.

Copy link
Member

@gmaclennan gmaclennan left a comment

Choose a reason for hiding this comment

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

@achou11
Copy link
Member

achou11 commented Aug 10, 2023

Need to remove this https://github.com/digidem/mapeo-schema/blob/master/scripts/generate.js#L24

@gmaclennan don't we need to keep that but update it to npx buf ...? Otherwise there's nothing that actually does the generation, from what I can tell

either update it or extract that command to the build:protobuf npm script

@gmaclennan
Copy link
Member

@gmaclennan don't we need to keep that but update it to npx buf ...? Otherwise there's nothing that actually does the generation, from what I can tell

oops yeah I had assumed the build:protobuf script did that, but now see that it just outputs the version. @sethvincent maybe make the npm script do the buf build? I moved it to execSync in the generate.js script under misguided assumptions.

package.json Outdated Show resolved Hide resolved
@sethvincent
Copy link
Contributor Author

Thanks! Updated to fix the things mentioned.

@achou11
Copy link
Member

achou11 commented Aug 10, 2023

@sethvincent now that you updated the npm script I think you can remove the line in the generate.js script

Other than that, lgtm!

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

Successfully merging this pull request may close these issues.

Install @bufbuild/buf via npm for installing buf CLI cache buf in github actions and/or use github_token
3 participants