This project is looking for an additional co-maintainer to be added to the team, to help out with QA and general improvement of the codebase for version 2.0.x.

If you're interested, create a patch or a merge request towards an open Code issue. If your code is good, you will be added to the team.

Note: We will only add a co-maintainer that's contactable, if you don't have the "Contact" tab on your profile enabled, you need to supply another means of contacting you.

Comments

gisle created an issue. See original summary.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Issue summary: View changes

Update.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented
Status: Active » Needs review

Hi Gisle,

Id like to try out as a co-maintainer. Which issues would you like me to solve first? I see 15 different issue open but most of them are already under review.

If you can let me know which issues to start with id be glad to give it a go!

Thanks in advance,

Rik

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Status: Needs review » Needs work

The issue with highest priority for me is #3225558: Take notify_subscriptions into account.

It would be great to get that bit correct.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented

Im going to explore the module today and ill try to fix the issue this week.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented
Status: Needs work » Needs review

I updated the issue and it looks to be working. The notify service can use a lot of cleaning up to make the code clearer and safer (Unused variables, naming conventions etc). I think it might be more productive for me to clean up the module a bit before solving more issues!

Id love to hear your feedback/opinion about my patch and ideas.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

Thank you for the patch!

I fully agree that the code of this project could to with some TLC. It was upgraded to Drupal 8 before I understood the architecture of Symfoni-based Drupal, and I would love to see it cleaned up.

I agree that it may be a good idea to do have it cleaned up before solving more issues.

I am quite busy these days, so I've added you as co-maintainer with permission to push to the repo, so that my tight schedule shall not be a speed-bump towards progress.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented

Hi @gisle can I be a co-maintainer? I worked on that issue (https://www.drupal.org/project/notify/issues/3301614) and I like so much I think I can help in possible updates of this module and I will learn a lot.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented

Thanks for adding me as a Maintainer gisle! Ill take a look at the first cleanup this weekend, ill try to keep it simple at first and do some major cleanups when it feels right.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

elber, I don't want to have too many co-maintainers that can push to the repo, because it makes coordination of resolving issues difficult.

I've just added rik.scholten to the team and he is planning to to do some much needed re-factoring of the code base in the near future, which means that the code base for the development branch is going to be unstable while this goes on.

I've noticed your work on #3301614: Use dependency injection for mail manager, and I made sure you received issue credit for that. However, you also needed a lot of coaching from MegaChriz to add the dependecy injection correctly, so I also think that it may be a bit too early to give you access as co-maintainer, where you can push directly to the project's repo without having the code you want to add reviewed in the issue queue first.

Your contributions are still very welcome, but I think it better to have them as patches or merge requests for the above reasons. I shall make sure you receive issue credit for any useful contribution you make.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented

Is there a more direct line where I can keep you up to date @gisle? I reworked the first bit of code from the Notify.php, its taking longer then expected but the code is cleaning up nicely. I'm aiming for the first commits this week but I have to test everything first.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

Thank you for your efforts. I don't think I want you to do anything more than refactoring the code base for the module.

I understand that this is going to take time. I acknowledge that the code was in a pretty bad shape.I appreciate your efforts, and that you're mindful about testing things carefully before committing.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented

I had a busy week but my first commit with rework is done. I got a few days off now so ill do one final test tomorrow and commit it to dev!

Took a bit longer then expected but I will make a lot of progress next week aswell.

Just wanted to update you since it was taking a bit longer then expected.

rik.scholten’s picture

rik.scholten
Primary language Dutch
commented

Hi Gisle, are the tests still credible in the module? I tried running them but im getting some conflicts on core versions. If the tests still need to be passed I shall fix this first before commiting!

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

AFAIK, the tests are still good. They do not cover all use cases, but those that exist should be passed.