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

Add CD for npm publish #44

Closed
lukasjhan opened this issue Jan 4, 2024 · 30 comments · Fixed by #89
Closed

Add CD for npm publish #44

lukasjhan opened this issue Jan 4, 2024 · 30 comments · Fixed by #89

Comments

@lukasjhan
Copy link
Member

Overview

OWF provide npm token for our repo. you can check it in discord channel.
We'll apply same way with https://github.com/openwallet-foundation/agent-framework-javascript/blob/main/.github/workflows/continuous-deployment.yml

But, I think it should be triggered by release

@lukasjhan lukasjhan added the ci/cd label Jan 4, 2024
@lukasjhan lukasjhan self-assigned this Jan 4, 2024
@cre8
Copy link
Contributor

cre8 commented Feb 15, 2024

One good approach I saw:

  • the code on main will always be published under the tag next
  • versions will be managed by creating a new release
  • release documents are auto generated by semantic release commits lik fix: ...

The downside via the manual release approach is that someone has to trigger it. So from my point of view I would prefer the approach you linked where we have an automated cicd pipeline. For code quality we can still force a 100% code coverage to be on the safer side ;)

@lukasjhan
Copy link
Member Author

That approach seems good. Currently I deploy it manually but OWF provide us a npm token, so next time, let's use it to publish automatically

I think triggering the release to the npm when we make a new release in github is better. :)
What do you think? @cre8

@lukasjhan lukasjhan removed their assignment Feb 15, 2024
@cre8
Copy link
Contributor

cre8 commented Feb 15, 2024

Before choosing a way, I would look at the used framework that should be used to manage a monorepo when we want to split it up.

E.g. Lerna has a lot of functions already for versioning and i would prefer to reuse these common approaches so we can focus to work on the lib and not the "managing stuff" around it.

@lukasjhan
Copy link
Member Author

I'll make a PR for integration with Lerna.

@lukasjhan
Copy link
Member Author

@berendsliedrecht @cre8

Based #68, now we can discuss the package names. I believe @openwalletfounction/* will be used since there is a token provided by OWF.

I made name in this format: sd-jwt-{sub package name}. so e.g. sd-jwt-type, sd-jwt-core.
I would appreciate it if you could share your thoughts please :)

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

the prefix with openwalletfoundation is fine for me, since owf is already taken.

also the approach with the general prefix sd-jwt is fine for me.

To make the versioning easier I would suggest to sync the version numbers over the packages. In this case when we release something, we have it like:

Version 1.1.0
sd-jwt-core:
- fixed bug foo bar

@lukasjhan
Copy link
Member Author

Version sync is great. If not, there will be a compatible issue between each versions

@lukasjhan
Copy link
Member Author

I guess there is no package named @owf/*.
How can I check owf is used or not? @cre8

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

I guess there is no package named @owf/*.
How can I check owf is used or not? @cre8

The name owf is already taken as an organisation, at least I was not able to create it

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

Any chances we can reuse the @sd-jwt that is used right now?

Other typescript projects from the owf like credo also use their own prefix.

@lukasjhan
Copy link
Member Author

we can use @sd-jwt, @berendsliedrecht has that org.

@berendsliedrecht
Copy link
Contributor

berendsliedrecht commented Feb 20, 2024

We can use the @sd-jwt, yes! Also, installing, or importing, multiple packages with the @openwalletfoundation can get quite annoying as it is fairly long :).

@lukasjhan
Copy link
Member Author

I like @sdj-wt too. :)

0.2.1 is the latest version of @sd-jwt packages

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

Do you think it makes sense to continue under the versioning we have at sd-jwt? I don't feel well saying we are at v2 or even v3 since the version before are not ready yet (and not active used).

Suggestion:

  • we migrate to @sd-jwt under 0.x and after we are happy to say "it's ready to be used" we publish version 1.0

@lukasjhan @berendsliedrecht how do you think about this?

@lukasjhan
Copy link
Member Author

That sounds good to me.

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

okay, I can look at the versioning stuff with lerna

@lukasjhan
Copy link
Member Author

To publish to npm with a github action, does an npm token(or something like that) need to be stored in github secret?

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

in case we want to publish to npmjs, yes, see https://docs.github.com/de/actions/publishing-packages/publishing-nodejs-packages

@lukasjhan
Copy link
Member Author

@berendsliedrecht Can you prepare an npm token so I can deploy this repo to @sd-jwt please?

@berendsliedrecht
Copy link
Contributor

@berendsliedrecht Can you prepare an npm token so I can deploy this repo to @sd-jwt please?

Yes! Just trying to think now of the best way to set this. @ryjones any tips? Do I just need to supply you the token and can you add the secret?

@cre8
Copy link
Contributor

cre8 commented Feb 20, 2024

Someone with admin privileges of this repo has to set the the token created by you @berendsliedrecht and store it as a secret in this repo. We can configure that it can only be used when the cicd runs on a protected branch.

@ryjones
Copy link
Member

ryjones commented Feb 20, 2024

please make owf an owner over on npm and I'll create and add the token.

@lukasjhan
Copy link
Member Author

@berendsliedrecht Could you invite me too? :)
[email protected]

@lukasjhan
Copy link
Member Author

I'll change the name in package.json today :)

@lukasjhan
Copy link
Member Author

done in #77

@lukasjhan
Copy link
Member Author

NPM_TOKEN: ${{secrets.NPM_TOKEN}}
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}

should change to

NPM_TOKEN: ${{ secrets.NPM_PUBLISH }}
NODE_AUTH_TOKEN: ${{ secrets.NPM_PUBLISH }}

@cre8
Copy link
Contributor

cre8 commented Feb 22, 2024

@lukasjhan will you make a PR or should I?

@ryjones
Copy link
Member

ryjones commented Feb 22, 2024

@lukasjhan @cre8 the secret is named NPM_TOKEN. Do I need to rename it?

@cre8
Copy link
Contributor

cre8 commented Feb 22, 2024

@ryjones @lukasjhan we do not need to rename it, I know why the ci is falling. It's because of this:
token: ${{secrets.GH_TOKEN}}

I fixed it on one of my branches, but didn't make a PR. So we do not need to update the secret name, but fix the workflow (I will do it tomorrow)

@lukasjhan
Copy link
Member Author

Okay :) It was my misunderstanding

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 a pull request may close this issue.

4 participants