Skip to content

Changed url for PereiraJS (Colombia) meetup#398

Merged
5 commits merged into
nodejs:masterfrom
pin3da:master
Mar 30, 2016
Merged

Changed url for PereiraJS (Colombia) meetup#398
5 commits merged into
nodejs:masterfrom
pin3da:master

Conversation

@pin3da

@pin3da pin3da commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

We don't use the meetup.com platform at this moment. It would be cool if you use http://pereirajs.org as referral page.

Let me know if we need to write the data in another format.

Thanks (:

@cronopio

cronopio commented Dec 1, 2015

Copy link
Copy Markdown

👍

@lpinca

lpinca commented Dec 1, 2015

Copy link
Copy Markdown
Member

The changes look good to me but they will be overwritten when the pull-meetup.js script is run.

@indexzero

Copy link
Copy Markdown
Contributor

Agreed @lpinca. I think the script @mikeal wrote will overwrite this.

@cronopio

cronopio commented Dec 1, 2015

Copy link
Copy Markdown

woot! Are we the only ones in the world NOT using meetup? 😱
BTW, I think is not good looking create an if stament in pull-meetup.js just to avoid Pereira been included there. Any other ideas?

@stevemao

stevemao commented Dec 1, 2015

Copy link
Copy Markdown
Contributor

From https://nodejs.org/en/get-involved/events/

If you aren't listed on those sites, or you are the organizer of a conference that is not listed, you can add it manually by editing events.md and sending a pull request to the website repo.

If it will be overwritten this is a problem?

@lpinca

lpinca commented Dec 2, 2015

Copy link
Copy Markdown
Member

I was thinking about something like this:

diff --git a/events/custom.json b/events/custom.json
new file mode 100644
index 0000000..5048232
--- /dev/null
+++ b/events/custom.json
@@ -0,0 +1,19 @@
+[
+  {
+    "id": 18205029,
+    "created": 1393477200000,
+    "link": "http://pereirajs.org/",
+    "description": "Comunidad de aprendizaje del lenguaje Javascript",
+    "organizer": {
+      "member_id": "",
+      "name": "",
+      "photo": {
+        "highres_link": "",
+        "photo_id": "",
+        "photo_link": "",
+        "thumb_link": ""
+      }
+    },
+    "members": 30
+  }
+]
diff --git a/events/pull-meetup.js b/events/pull-meetup.js
index 05749c1..d1a3391 100644
--- a/events/pull-meetup.js
+++ b/events/pull-meetup.js
@@ -1,7 +1,9 @@
 var request = require('request').defaults({json: true, headers: {'user-agent': 'pull-meeting-0.1'}}),
   url = 'https://api.meetup.com/2/groups',
   auth = process.env.MEETUP_TOKEN,
+  merge = require('lodash.merge'),
   qs = require('querystring'),
+  custom = require('./custom'),
   yml = require('./yaml-sync'),
   opts =
   { topic: 'nodejs',     category: 34,     upcoming_events: true,     key: auth
@@ -29,6 +31,12 @@ function finish (events) {
     }
     var region = yml.getRegion(countryMap[event.country])
     if (!region.meetups) region.meetups = []
+    custom.some(elem => {
+      if (event.id === elem.id) {
+        event = merge(event, elem)
+        return true
+      }
+    })
     clean(event)
     yml.replace(region.meetups, 'name', event.name, event)
   })
diff --git a/package.json b/package.json
index fb04c03..9e9f611 100644
--- a/package.json
+++ b/package.json
@@ -30,6 +30,7 @@
     "html-to-text": "^1.5.0",
     "js-yaml": "^3.4.5",
     "junk": "1.0.2",
+    "lodash.merge": "^3.3.2",
     "map-async": "0.1.1",
     "marked": "0.3.5",
     "metalsmith": "2.1.0",

so you can simply add your customized event in custom.json and in this way changes are not overwritten. The problem is that the event id is not shown and has to be extracted from meetup api.
Truthfully, I think this is cumbersome so if you have better ideas please share.

@mikeal

mikeal commented Dec 3, 2015

Copy link
Copy Markdown
Contributor

When we pull data in from another source we consider that resource the source of truth for that information. If we want the local copy to be the source of truth we should add a flag in the markdown and make sure our scripts honor it and don't overwrite it. I don't think we should try and merge information we have locally with information we pull from another source, that's going to get messy :)

@stevemao

stevemao commented Dec 4, 2015

Copy link
Copy Markdown
Contributor

Can we just simply include two md files: one is generated from the script and the other is not?

@pin3da

pin3da commented Dec 16, 2015

Copy link
Copy Markdown
Contributor Author

I tried to do what @mikeal said : db53e29

Tell me if I need to change something.

cc @cronopio

Comment thread events/yaml-sync.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we reuse exports.getRegion?

@fhemberger

Copy link
Copy Markdown
Contributor

@nodejs/website @pin3da What's the status of this PR?

- Using getRegion now
- Identifying meetups by region, city and name

Conflicts:
	events/yaml-sync.js
@pin3da

pin3da commented Mar 30, 2016

Copy link
Copy Markdown
Contributor Author

@fhemberger I just fixed the conflicts. I also added the @lpinca 's suggestions.

Let me know if there is something missing/wrong.

Thank you (:

Comment thread events/pull-meetup.js Outdated
if (!region.meetups) region.meetups = []
clean(event)
yml.replace(region.meetups, 'name', event.name, event)
if (!yml.isSoT(region, event.city, event.name)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use region.meetups here instead of region?

@lpinca

lpinca commented Mar 30, 2016

Copy link
Copy Markdown
Member

Left some comments but LGTM.

@lpinca

lpinca commented Mar 30, 2016

Copy link
Copy Markdown
Member

All good here, 🚢 ?

@ghost

ghost commented Mar 30, 2016

Copy link
Copy Markdown

aye

@ghost ghost merged commit ebf7f49 into nodejs:master Mar 30, 2016
@lpinca

lpinca commented Mar 30, 2016

Copy link
Copy Markdown
Member

Thank you @pin3da!

@pin3da

pin3da commented Mar 30, 2016

Copy link
Copy Markdown
Contributor Author

Thank you all 😺

lpinca added a commit that referenced this pull request Apr 9, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants