novuhq
Novu Bulksms Integration
What kind of change does this PR introduce?
- [ ] Bug
- [x] Feature
- [ ] Docs
- [ ] Other(s)
Why was this change needed?
#1767
Other information (Screenshots)
@satyajeet0308 Not 100% sure maybe i'm just missing something from the PR. But I actually can't find the package itself for bulk SMS, I only see the provider files.
You would need to use the npm generate provider script we have to scaffold It for you with our needed files and structure. Could you take a look at this please?
@scopsy while commenting by mistakenly I have closed the pull request so I have reopened the same
@scopsy I have already gone through the command npm run generate:provider and I have also followed all the steps for this that is written in the documentation part, but still if I am missing any thing can you just guide me how to do or what to do so that I can do the modifications ?
@satyajeet0308 Not 100% sure maybe i'm just missing something from the PR. But I actually can't find the package itself for bulk SMS, I only see the provider files.
You would need to use the
npmgenerate provider script we have to scaffold It for you with our needed files and structure. Could you take a look at this please?
Hi @scopsy I have already gone through the command npm run generate:provider and I have also followed all the steps for this that is written in the documentation part but it has not created any package for me, but still if I am missing any thing can you just guide me how to do or what to do so that I can do the modifications ?
bulk-sms
@p-fernandez I have also changed the folder name from bulkSms to bulk-sms and I have aslo pushed the same
Hi @satyajeet0308 any updates? Let me know if you had a chance to see @p-fernandez comments, would love to merge this once finished 🎉
Hi @satyajeet0308 any updates? Let me know if you had a chance to see @p-fernandez comments, would love to merge this once finished 🎉
@scopsy Sorry for the delay yes I have gone through the comments and I am working on it
@p-fernandez can we merge this? or @satyajeet0308 needs to update something?
@p-fernandez can we merge this? or @satyajeet0308 needs to update something?
Last change requests haven't been addressed yet, unfortunately. 😞
@p-fernandez can we merge this? or @satyajeet0308 needs to update something?
Last change requests haven't been addressed yet, unfortunately. 😞
@p-fernandez can you please re-review it, have done certain changes hope this time we can finally add this provider.
@satyajeet0308 unfortuently I still see that the bulk-sms package is not correct :( It might be because you already generated a folder for it. So maybe try removing the folder and running generate provider script again? Another thing you can do, is take another SMS example package and copy-paste it with modifying the relevant code places to be for bulk-sms
@scopsy thanks for the review. Have gone through the comments will do the needful.
@satyajeet0308 unfortuently I still see that the bulk-sms package is not correct :( It might be because you already generated a folder for it. So maybe try removing the folder and running generate provider script again? Another thing you can do, is take another SMS example package and copy-paste it with modifying the relevant code places to be for bulk-sms
@scopsy Have created a new folder of bulksms please review it. If anything is required please let me know
Would really love to merge this, @satyajeet0308 could you help with resolving conflicts? (For pnpm just run pnpm i locally)
Would really love to merge this, @satyajeet0308 could you help with resolving conflicts? (For pnpm just run pnpm i locally)
Hi @scopsy have resolved the pnpm conflicts please check and tell me if anything is required from my side
Closing for inactivity. Feel free to reopen and apply the changes suggested.