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

Change URL format in getInfo #279

Merged
merged 2 commits into from Jan 30, 2018
Merged

Change URL format in getInfo #279

merged 2 commits into from Jan 30, 2018

Conversation

FireController1847
Copy link
Collaborator

When we set the URL on the return for info, it includes every parameter in the video. This is fine until you start wanting to compare this video to a saved URL you may have (This is just one use case). Using a standardized format, we should instead just put the raw URL. If requested, we could also put the parameters in a separate variable or even the URL with parameters in a second variable.

@codecov
Copy link

codecov bot commented Jan 30, 2018

Codecov Report

Merging #279 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #279   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines         594    594           
  Branches      148    148           
=====================================
  Hits          594    594
Impacted Files Coverage Δ
lib/info.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48217a9...40166a3. Read the comment docs.

Copy link
Owner

@fent fent left a comment

Choose a reason for hiding this comment

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

Good idea!

lib/info.js Outdated
@@ -80,8 +80,8 @@ module.exports = function getInfo(link, options, callback) {
// Get related videos.
related_videos: util.getRelatedVideos(body),

// Give the canonical link to the video.
video_url: url,
// Give the raw link to the video.
Copy link
Owner

Choose a reason for hiding this comment

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

How about standard link?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright

@fent fent merged commit bcddc67 into fent:master Jan 30, 2018
@FireController1847 FireController1847 deleted the patch-1 branch January 30, 2018 06:19
fent pushed a commit that referenced this pull request Jan 30, 2018
* Update info.js
* Raw -> Standard
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.

None yet

2 participants