-
Notifications
You must be signed in to change notification settings - Fork 44
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
More maintainers for agenda-rest + npm package #74
Comments
@keyvan-m-sadeghi : That's awesome. I am in !! |
@sampathBlam great, can I have your NPM username? |
I really don't get why more users is the right idea here. I would highly suggest you reconsider this and instead use Github actions to publish the package. If any of the users get their accounts compromised they'll be able to publish, automated publishing avoids this. |
Very good suggestion @OmgImAlexis, I wasn't aware that GitHub actions supports this use case. Also I noticed that I actually don't have admin rights over the repo. Could you please help with setting this up with best practices? |
Don't have a great deal of spare time but this should help and I'm free to answer questions you have. I'd suggest making the publish happen on new commit to master that way it'll only happen when we merge a PR, or push directly to master. |
I'm available too. I can help with setting up Github actions. |
Great @geosp, I really haven't found the time to go about it. @OmgImAlexis appreciate if you can give @geosp write access to the repo, I can do the npm side. |
@keyvan-m-sadeghi and @OmgImAlexis I will do my best to help. I notice that the build was failing for a long time so I took the time to fix that too in my latest PR #93 . Thank you. |
Looks good @geosp, what's your username on npm? |
It is gffajardo. |
@keyvan-m-sadeghi and @OmgImAlexis Any word regarding access or PR #93 ? I know you are busy, please let me know. Thank you. |
Invite should have been sent on GitHub sorry for it taking so long. |
Thank you @OmgImAlexis |
@keyvan-m-sadeghi Any word in regards to NPM access? |
Sorry @geosp been away for a few days. I just checked npm and noticed I no longer have admin access there. @OmgImAlexis can you give write access to @geosp on npm? Username is Great work on refactoring @geosp 🎉 👍 |
Thank you @keyvan-m-sadeghi. I want to use this PR to test deploying to npm using GitHub actions. I assume you approve of the changes in #94 . |
Yeah all good 👌 |
@geosp I'm trying to give you access through the "teams" feature of npm, check your email for an invite from "assister" org and let me know if that gives you write access to the |
@geosp give me a hint when you accepted the invite, apparently there's an additional step I need to take before you actually get write access (adding you to |
@keyvan-m-sadeghi Thank you for your assistance. I'm in the org now. |
@geosp done, you now have write access to the npm package. You need to enable 2FA in your npm account to publish though. Feel free to merge pull requests as they come in, welcome aboard! |
Thank you, guys. I got really busy there for a while. I will be working on the GitHub-actions task now. |
@keyvan-m-sadeghi and @OmgImAlexis Hello guys. I created an issue for the implementation of GitHub actions and a PR to resolve the issue. Please take look at PR #96 . |
@OmgImAlexis Can you please add the NPM security token to GitHub so that the workflow is able to publish the library to NPM. Thank you so much for your help. |
I was able to publish the new version manually, but it will be nice to have it done automatically. |
Sorry that I've been away @geosp, it is indeed nice to automate publishing, hope @OmgImAlexis reads this as I don't have admin rights for the repo to give you the keys. |
Also thanks for the great work on updating everything @geosp 🎉 |
Would be cool if we can have a couple more maintainers with write access to the repo as well as the npm package for faster publish, as it seems I've been a bit neglectful in this regard.
Anyone feeling like it, your help is very much appreciated.
I'd need npm usernames.
@geosp
@sampathBlam
The text was updated successfully, but these errors were encountered: