- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 6.4k
 
Update the list of global events #625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
          
  | 
    
| 
           It wasn't intentional and I noticed the empty lines issue.  | 
    
| 
           I'll try to fix this later or tomorrow.  | 
    
| 
           The new line issue seems to be a bug in  const data = yaml.safeLoad(contents);
const dump = yaml.safeDump(data); // => unwanted new lines.See nodeca/js-yaml#221, nodeca/js-yaml#222. Not sure what to do about it. Those issues seem to be fixed in master and it looks like that a new version of  Maybe it makes sense to rebuild   | 
    
| 
           Hmm, in the meantime, how about something like   | 
    
| 
           @fhemberger I tried that, not so useful, the problem is that the major part of the new lines are added when data is dumped with  We can probably work around that issue using the  That's why I think it's probably better to regenerate   | 
    
08b1567    to
    d4931de      
    Compare
  
    | 
           I've updated the pr to work around the  There is a lot of diff noise but I tried to make the review process a little "easier" by splitting the changes over multiple commits. If we wait for the next version of   | 
    
| if ( | ||
| title.includes('find a tech job') || | ||
| title.includes('nodeschool') || | ||
| / mongodb (?:user group|meet ?up)$/.test(title) || | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not title.includes('mongodb')?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there are some MongoDB+Node.js events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are also a lot of irrelevant events like Angular Ember etc but it's kinda hard to have ONLY relevant Node.js events. This a flaw in Meetup categories and topics.
| 
           They will be updated yes, but not deleted. New events will be added.  | 
    
| 
           Great, thank you!  | 
    
This works around a js-yaml issue where spurious new lines are incorrectly added when serializing an object. See nodeca/js-yaml#222.
Some NodeSchool events don't have a `country` property, so we should check if it is defined before updating the `location` property.
d4931de    to
    ba8336d      
    Compare
  
    | 
           @fhemberger I've added the requested comments, let me know if they should be changed/improved.  | 
    
| 
           @lpinca LGTM! 👍  | 
    
Follow up of #614.
This updates
events.mdto fix the Spain region issue.I've used my own API key for meetup.com and no API key for node-geocoder (it seems to work fine).