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.

Sign up for GitHub

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

Jump to bottom

Make contains URL functions match between client and server #340

Merged
merged 3 commits into from
May 9, 2016
Merged

Make contains URL functions match between client and server #340

merged 3 commits into from
May 9, 2016

Conversation

david-a-wheeler
Copy link
Collaborator 

The client and server need to agree on what's required to
have a URL in a justification.  Otherwise the subtle difference
could produce confusing results in rare cases.  Also, add tests for them.

@david-a-wheeler
Make contails URL functions match between client and server
91ebe71 
  The client and server need to agree on what's required to
  have a URL in a justification.  Otherwise the subtle difference
  could produce confusing results in rare cases.

Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
@david-a-wheeler
Add more tests of Ruby mechanisms for examining URLs
ae4459b 
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
dankohn
dankohn reviewed May 9, 2016
View reviewed changes
test/models/project_test.rb
refute Project.new.send :contains_url?, 'mailto://mail@example.org'
refute Project.new.send :contains_url?, 'abc'
refute Project.new.send :contains_url?,
'See also http://x for more information.'
refute Project.new.send :contains_url?, 'www.google.com'
Copy link
Contributor

Choose a reason for hiding this comment

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

.send is only necessary for testing private methods. Could you please change all of these to the form:

refute Project.new.contains_url? 'mailto://mail@example.org'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure.

@david-a-wheeler
Clean up URL tests
3cdb41d 
Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
@david-a-wheeler
Copy link
Collaborator Author

Ok to merge?

@dankohn
Copy link
Contributor

dankohn commented May 9, 2016

Yes, please merge.

@david-a-wheeler david-a-wheeler merged commit c65f5c8 into master May 9, 2016
@dankohn dankohn deleted the url2 branch May 15, 2016 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@david-a-wheeler @dankohn