search

backdrop-contrib / sendgrid_integration

GNU General Public License v2.0
0 stars 1 forks source link

Updated To Not Need Composer #1

Closed ElusiveMind closed 8 months ago

ElusiveMind commented 8 months ago

Hey @joelsteidl - Unaware of this port, I did a port of SendGrid Integration for Backdrop CMS as well. Our versions have similarities, but I think have too many differences to be merged. Would you be amenable to having the version I have worked on replace this one? Especially since this edition has not been released yet?

I appreciate the work you have done and we were definitely on the same page regarding about 85% of the work to be done for SendGrid. But I think the changes between the two repos would be too significant to attempt to merge them but I would love to get your thoughts. For references, here is the link to my repo

https://github.com/ElusiveMind/sendgrid_integration

joelsteidl commented 8 months ago

Hi @ElusiveMind That would be great. What do you need from me to help?

laryn commented 8 months ago

Cross-posting essentially what I wrote in Zulip here:

@ElusiveMind, can you repurpose this issue to add yourself in the README as maintainer, so we have a record of the transfer? And make a PR to the current 1.x-1.x branch with that change? Once @joelsteidl merges that PR I can help adjust permissions on the repo.

ElusiveMind commented 8 months ago

@laryn - I am issuing a PR with my updates to the module and readme. This should bring the existing repo up to date with my work. This is a big PR because it will have the vendor dependency files in it, but you should be able to code review if desired and just ignore the vendor folder.

joelsteidl commented 8 months ago

@ElusiveMind I think the initial PR just adds yourself as the maintainer in the README and then you can merge in your version.

ElusiveMind commented 8 months ago

Hey @joelsteidl - I just submitted another one with that change

laryn commented 8 months ago

@ElusiveMind You should have permissions at this point -- I adjusted them since you've been merged into the README as a maintainer. I haven't done a full code review but did notice a number of comments where you've got to update "Drupal" to "Backdrop" yet:

CleanShot 2023-10-17 at 17 57 14@2x

ElusiveMind commented 8 months ago

@laryn - Thanks for pointing those out. Looks like I missed a few. I'll be taking a refresher look at this before I merge it likely tonight or tomorrow.

ElusiveMind commented 8 months ago

I've updated the PR with the replacement of Drupal references with Backdrop. I will merge on 10/19 if no objections. Then module will see initial release.

ElusiveMind commented 8 months ago

merged