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

Add source type 'web' so that siphon can suck up information from a website #214

Merged
merged 4 commits into from
Jan 2, 2019

Conversation

patricksimonian
Copy link
Contributor

@patricksimonian patricksimonian commented Dec 20, 2018

Summary

Added another 'sourceType' to siphon called web

A web source type and takes a url property as its sole sourceProperties

The url is unfurled and then created into a graphql node

Changes

@patricksimonian patricksimonian added task: enhancement New feature or request status: not ready for merge this pr is not ready to be merged, do not merge! Reviewer Wanted labels Dec 20, 2018
@patricksimonian patricksimonian self-assigned this Dec 20, 2018
@repo-mountie
Copy link
Contributor

repo-mountie bot commented Dec 20, 2018

@patricksimonian When a pull request (PR) is over the 200-400 line range (of change) the ability of the reviewer to effectively detect issues quickly diminishes. Your PR is over the preferred length of 300 lines set for this repo. While this cannot always be avoided, please be mindful of the reviewer's time by organizing your work into smaller, more manageable PRs.

Here are a few thoughtful articles on what a good PR should look like:

The Anatomy of a Perfect Pull Request

Optimal Pull Request Size

Best Practices for Code Review

Pro Tip

  • Add the command /bot-ignore-length in the body (preferably as the last line) of a PR if you have a legitimate reason for lengthy PR.

@patricksimonian patricksimonian removed the status: not ready for merge this pr is not ready to be merged, do not merge! label Dec 21, 2018
Refactor unfurl

update tests due to unfurl function refactor

add tests for unfurlWebURI fn

.

complete web source type

unfurlWebURI also combines generic unfurl data

fix lint
Copy link
Contributor

@ShellyXueHan ShellyXueHan left a comment

Choose a reason for hiding this comment

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

all good!

@patricksimonian patricksimonian merged commit e957479 into master Jan 2, 2019
@patricksimonian patricksimonian deleted the imp/siphon-source-type-web branch January 2, 2019 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants