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

Detect discourse forum #2

Merged
merged 5 commits into from
Oct 20, 2018
Merged

Conversation

bekicot
Copy link
Contributor

@bekicot bekicot commented Jan 6, 2018

This patch detect discourse forum

resources/chat-types.js Show resolved Hide resolved
index.js Outdated
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

undesirable removal of whitespace

bekicot added a commit to bekicot/gci-leaders that referenced this pull request Jan 6, 2018
bekicot added a commit to bekicot/gci-leaders that referenced this pull request Jan 6, 2018
bekicot added a commit to bekicot/gci-leaders that referenced this pull request Jan 8, 2018
index.js Outdated
const parse = require('url').parse;
const rp = require('request-promise');
const CHAT = require('./resources/chat-types');
const cherio = require('cherio');
Copy link
Owner

Choose a reason for hiding this comment

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

Could we keep this like it was, please?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You mean, no re-indentation ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

also needs cheerio here.

index.js Show resolved Hide resolved
index.js Outdated
@@ -88,6 +92,10 @@ async function fetchChatLink(url) {
return { type: CHAT['IRC'], url: response.request.href };
}

if(patterns.discourse.exec(generator)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Follow the formatting of previous RegEx matches. Should be something like this:

const discourseMatches = patterns.discourse.exec(generator)
if (discourseMatches) {
  //...
}

Copy link
Owner

Choose a reason for hiding this comment

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

Also please move this above the IRC matching above as it's more similar to the RegEx ones.

package.json Outdated
@@ -7,6 +7,7 @@
"author": "Andrew Dassonville <dassonville.andrew@gmail.com>",
"license": "MIT",
"dependencies": {
"cherio": "^1.0.0-rc.2",
Copy link
Owner

Choose a reason for hiding this comment

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

Luckily the person maintaining cherio is aliasing it to the correct dependency, cheerio, but we should be using the official name of this dependency: https://www.npmjs.com/package/cheerio

@jayvdb
Copy link
Collaborator

jayvdb commented Jan 27, 2018

ping @bekicot

@jayvdb
Copy link
Collaborator

jayvdb commented Sep 10, 2018

ping @bekicot , @wisn , we need this for the next GCI.

@wisn
Copy link

wisn commented Sep 10, 2018

@jayvdb Do I need to take over this PR?

@bekicot
Copy link
Contributor Author

bekicot commented Sep 15, 2018

@wisn Yes pls.

@andrewda andrewda merged commit e5c7b9e into andrewda:master Oct 20, 2018
@andrewda andrewda added the gci label Oct 20, 2018
@jayvdb
Copy link
Collaborator

jayvdb commented Oct 20, 2018

Thanks @andrewda

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants