-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix social media validation for Twitter #9053
Conversation
lib/cocoapods/validator.rb
Outdated
@@ -450,7 +450,7 @@ def validate_screenshots(spec) | |||
# Performs validations related to the `social_media_url` attribute. | |||
# | |||
def validate_social_media_url(spec) | |||
validate_url(spec.social_media_url) if spec.social_media_url | |||
validate_url(spec.social_media_url, 'Not a Browser') if spec.social_media_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be 'CocoaPods' or is this standard practice for unknown/placeholder user agents?
lib/cocoapods/validator.rb
Outdated
@@ -416,7 +416,7 @@ def perform_extensive_subspec_analysis(spec) | |||
|
|||
# Performs validation of a URL | |||
# | |||
def validate_url(url) | |||
def validate_url(url, user_agent=nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit formatting: user_agent = nil
I think Rubocop will catch that anyway.
I'll take a look at the broken test specs tomorrow. |
@dnkoutso This is all set now, if you'd like to review again. |
@abbeycode the Core PR has been merged please update the |
Also please squash into a single commit. |
@dnkoutso I'm curious what the reason for squashing is, since GitHub lets you squash when you merge. I'm happy to do it, but I was curious if there's a reason that applies to the projects I maintain. |
No big reason its just that I forget to update the merge mode :) sorry |
All set, once the checks complete. |
Looks like there was no squash anyway i will make sure its squashes into a single commit once merging. |
Using change from CocoaPods/Core#571 to address Issue #9049