Skip to content
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

Changed url for PereiraJS (Colombia) meetup #398

Merged
5 commits merged into from
Mar 30, 2016
Merged

Changed url for PereiraJS (Colombia) meetup #398

5 commits merged into from
Mar 30, 2016

Conversation

pin3da
Copy link
Contributor

@pin3da pin3da commented Dec 1, 2015

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
Copy link

cronopio commented Dec 1, 2015

👍

@lpinca
Copy link
Member

lpinca commented Dec 1, 2015

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

@indexzero
Copy link
Contributor

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

@cronopio
Copy link

cronopio commented Dec 1, 2015

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
Copy link
Contributor

stevemao commented Dec 1, 2015

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
Copy link
Member

lpinca commented Dec 2, 2015

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
Copy link
Contributor

mikeal commented Dec 3, 2015

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
Copy link
Contributor

stevemao commented Dec 4, 2015

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

@pin3da
Copy link
Contributor Author

pin3da commented Dec 16, 2015

I tried to do what @mikeal said : db53e29

Tell me if I need to change something.

cc @cronopio

@@ -16,6 +17,22 @@ exports.getRegion = function (region) {
return reg
}

exports.isSoT = function(region, city) {
for (var reg in store.regions) {
Copy link
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
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
Copy link
Contributor Author

pin3da commented Mar 30, 2016

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

Let me know if there is something missing/wrong.

Thank you (:

@@ -29,7 +29,9 @@ function finish (events) {
const region = yml.getRegion(countryMap[event.country])
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
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
Copy link
Member

lpinca commented Mar 30, 2016

Left some comments but LGTM.

@lpinca
Copy link
Member

lpinca commented Mar 30, 2016

All good here, 🚢 ?

@ghost
Copy link

ghost commented Mar 30, 2016

aye

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

lpinca commented Mar 30, 2016

Thank you @pin3da!

@pin3da
Copy link
Contributor Author

pin3da commented Mar 30, 2016

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