search

snowplow-referer-parser / referer-parser

Library for extracting marketing attribution data from referrer URLs
http://snowplowanalytics.com
359 stars 150 forks source link

Missing PR Reviews in node-referer-parser since a year #218

Open julianbei opened 3 years ago

julianbei commented 3 years ago

Hello Community.

I am reaching out here because this module seems to be actively maintained. The corresponding node-js module has two open PRs with one PR having its first birthday soon. The whole Project seems to be abandoned. The last commit is from the 22 May 2018.

The two PRs are:

I am really looking forward for some Reviews. I could take over maintenance if necessary. We use that module in production and have a big interest in it being regularly updated and security hardened and fixed.

Cheers Julian

chuwy commented 3 years ago

Hey @julianbei! We owe you a big apology for not paying attention to this. And I think there's a lot of things we need to change about maintainance of these repositories. We don't have much resources to maintain all client libraries except Scala one, but we're still interested in their development. We'll grant you Write access to nodejs-referer-parser (anyone else from your team?) and will start looking for maintainers for other ones. It would much appreciated if your contributions went through the formal PR/review process.

julianbei commented 3 years ago

Hi @chuwy thanks for coming back to this. I know what it means to maintain a landscape with multiple repositories per developer, so no apology needed. In addition to me, I have two colleagues from my Data Engineering team that could help in maintaining this repo: https://github.com/Joey92 and https://github.com/avakarev

Thanks for making this possible. We are always happy to help.

chuwy commented 3 years ago

Added @Joey92 and @avakarev as well. Let us know if we can help with something else, Julian and thanks again for the work you're doing.

julianbei commented 3 years ago

Thanks for adding them. Always a pleasure. we can close this here then. Last question: Should we just add CI and stuff to the repo? It is missing a lot of auxillary.

chuwy commented 3 years ago

Should we just add CI and stuff to the repo? It is missing a lot of auxillary.

If possible - that would be great, @julianbei! @paulboocock can add necessary NPM credentials as GH secrets.

chuwy commented 3 years ago

Ah, Paul is on vacation - just noticed it. I also can do that, just let me know what should be added.

julianbei commented 3 years ago

Thanks! We will discuss internally again. We thought of doing everything with github actions directly.

chuwy commented 3 years ago

We thought of doing everything with github actions directly.

Yeah, I think this is a common approach these days. Just in case here's some prior art in another JS project: https://github.com/snowplow/snowplow-nodejs-tracker/commit/7ad12c68348807e5c1bd7e12efca8705da6aee85, which requires GITHUB_TOKEN and NPM_TOKEN

paulboocock commented 3 years ago

We're also moving to scoped packages so would be good to take this opportunity to publish as @snowplow/referer-parser

Can probably bump to 1.0.0 too.

I'm on vacation but still around, happy to help get this updated and published, not much to do out of the house at the moment 😅

paulboocock commented 3 years ago

@julianbei The NPM_TOKEN secret is now available in https://github.com/snowplow-referer-parser/nodejs-referer-parser

I'm happy to review any nodejs PRs before the first release. Please just @ me in any PRs you would like me to review, I won't review the current open ones yet in case you want to update them based on conversations here but please just tag me when you feel it is ready to go.

julianbei commented 3 years ago

Alright. We will have a closer look in the upcoming days. Pre X-mas is a tough time + corona. Thanks for the hints and support. @paulboocock we will let you know as soon as we are ready to roll. Much appreciate to go to scoped packages and lift the quality overall in order to roll with 1.0.0 there directly. We might create a release candidate branch in order to keep the tasks in smaller PRs wich are easier to review but do not pollute master with unfinished work.

julianbei commented 3 years ago

BTW @chuwy we have some more packages we can share over the next year (we need to clarify some legal stuff internally first)

chuwy commented 3 years ago

Hey @julianbei! That's a quite an impressive list. Do you want to schedule another call to discuss these maybe?

julianbei commented 3 years ago

Yes lets do this next year. I just want this year to pass. :-) Maybe we have more to share than just code.

mcadamm4 commented 3 years ago

Hey guys, just wondering did this fall through or are there any plans to pick up maintenance of the nodejs parser and other repos again?

paulboocock commented 3 years ago

We haven't managed to get to this within the Snowplow eng team and I don't think we will in the short term. We still do want to support these repositories, it's a case of prioritising our efforts at the moment though. I'll comment on the nodejs-referer-parser PRs too.