Feeds: Meetup is a set of parsers that build on Feeds to import data from Meetup.com API calls.

Currently, the project supports XML feeds for events, groups, and venues. Support is planned for more API methods, as well as JSON, RSS, and Atom formats. I modeled my code after the Feeds: YouTube module.

I have already run through automated testing and have had several peers review my code.

Project Sandbox
https://drupal.org/sandbox/joshsimmons/2070677

Git Repo
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/joshsimmons/2070677.git feeds_meetup

Sample API call & key for testers
For test imports with my parsers, Meetup API calls are formed like so: https://api.meetup.com/2/groups.xml/?key=[KEY]&sign=true&category_id=34
... and with a key (testers can use mine): 377c682266112e355a7a435d4dd2021

Screenshot

Reviews of other projects

Comments

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue summary: View changes

Added a screenshot!

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue summary: View changes

D.O complaining about non-local image, even tho it is local. converted screenshot to a relative URL

PA robot’s picture

PA robot CreditAttribution: PA robot commented

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

PA robot CreditAttribution: PA robot commented
Issue summary: View changes

derp. D.O really has trouble knowing what is local and not. converted screenshot IMG tag to a link.

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue summary: View changes

switch to non-maintainer git clone command

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue tags: +PAreview: review bonus

Review bonus, hooray :D

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue summary: View changes

review bonus! huray

alexmoreno’s picture

alexmoreno CreditAttribution: alexmoreno commented

Hi bluesomewhere. Thanks for your effort with this module.

It's a minor thing, but your README file does not include all the requirements (job scheduller and ctools):
Requires: Feeds (disabled), Ctools (missing), Job_scheduler (missing), Feeds XPath Parser (disabled)

but I think it is all right as they are dependencies from the other two modules, feeds and feeds xpath.

Once installed you have this issue in some pages like when listing modules:

Notice: Undefined variable: ret_val in feeds_meetup_help() (line 46 of /var/www/drupal7/sites/all/modules/feeds_meetup/feeds_meetup.module).

If the switch does not enter in the admin/help#feeds_meetup case the variable will never have a value, and will be returning an uninitialized variable.

This is more a question. In line 186, if $entry->venue is not set, can it happen that the $entry->fee is set? If not, maybe you should include this condition inside the other if. Even more, to simplify the code and improve readability I would create a function with this group of ifs. Let me know what you think.

Apart from that, the code is simple, maybe short and inside the limits of the minimal requirements, but looks good from my side.

Thanks again.

alexmoreno’s picture

alexmoreno CreditAttribution: alexmoreno commented

in fact, the code in the .inc file looks like a third part library, doesn't it? In that case the module would be not be long enough to be approved, you'd probably need to extend it a bit.

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented

Thank you for the review, urwen!

  • dependency list updated, erring on side of caution, in .info and README
  • help function updated to nix undefined variable notice
  • cleaned up variable setting code if statements, feel like there may be a better pattern but unaware of it
  • if statements commented for clarity. (a note: fee may be specified separately from venue, tho rarely)
  • inc file is inspired from Feeds: YouTube and Feeds XPath Parser

Only issue I'm not sure how to address is the amount and/complexity of the code, as I definitely want to get full project access, rather than a one time project promotion. For now, I'll try to address the issue by adding a parser for Groups, Venues, and Members (other entities available via Meetup API).

alexmoreno’s picture

alexmoreno CreditAttribution: alexmoreno commented

Have also in mind that if your code includes a third party library, it may not be allowed to be into drupal. There are ways to add them, just having all settled to work in a folder and giving the user instructions about how and where installing it (like sites/libraries)

Thank you

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented

To address complexity and length, I've continued down my planned roadmap and added a Groups and Venues parser, and taken the helper function out of the Events parser and moved it into the module. Photos and Members are forthcoming.

And no worries re: third party code, this is all GPL'd Drupal code :)

Thanks again, urwen!

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue summary: View changes

reviewed another project application, just because

alexmoreno’s picture

alexmoreno CreditAttribution: alexmoreno commented

Hi bluesomewhere,

If I am right, the manual reviews should be a bit deeper than the ones you have done here, and you cannot include just an automated review. A manual review implies that you have installed the module, and most important you have read througfully the code, understood what coder did and which part would be wrong. I know it is difficult, but believe me, my first review was january this year, and I am still learning good practices, so this work is really important, not just one thing to get approved your module.

Also, if you review a module that is nearly receiving the approval, obviously this is not going to show a lot about your coding skills. I think I should remove you the bonus review tag, but I will wait for other dev with more experience to confirm that.

Thank you for your time.

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue tags: -PAreview: review bonus

Hi urwen,

I can appreciate that the nature of Bonus Review is more nuanced than I initially thought – I understood it primarily as a way to accelerate the rate at which project applications get reviewed. I was aware that my reviews were shallow and more focused on process than code, thus the +1. In any case, I will voluntarily remove the tag and get in line.

Also, I'm not sure this module offers an opportunity for me to demonstrate my coding ability, even after I add support for additional formats and API calls.

Perhaps this project is a good candidate for one time promotion, though I aspire to more. I'll leave that to you and the community to decide :)

Thanks for walking me through this.

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented
Issue summary: View changes

dropped needless dependency on Feeds XPath Parser, updated current feature list, as well as planned features

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Assigned: Unassigned » mlncn
Status: Needs review » Reviewed & tested by the community

Sorry for the delay. Make sure to review more project applications and get a new review bonus and this will get finished faster.

manual review:

  • The parse() method seems to be the same for all your parser classes, why the duplication? Just use a common base class?

Otherwise looks RTBC to me. I think the code in the repository is quite simple but still enough to approve you as git vetted user.

Assigning to mlncn as he might have time to take a final look at this.

bluesomewhere’s picture

bluesomewhere CreditAttribution: bluesomewhere commented

Awesome, thank you for the review! I took your advice and created a base class with the parse() method.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Reviewed & tested by the community » Fixed

no objections for more than a week, so ...

Thanks for your contribution, bluesomewhere!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

added a sample meetup API call and key to make testing easier