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
Use metadata uri instead of linkset for github links #2316
Conversation
app/views/rubygems/_aside.html.erb
Outdated
@@ -1,6 +1,8 @@ | |||
<div class="gem__aside l-col--r--pad"> | |||
<% if @rubygem.linkset.present? && github_link = link_to_github(@rubygem) %> | |||
<%= render partial: "rubygems/github_button", locals: { github_link: github_link } %> | |||
<%= "hiiii"%> |
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.
✂️
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.
thanks @simi. updated.
fd6a863
to
d17baed
Compare
app/views/rubygems/_aside.html.erb
Outdated
<% if @rubygem.linkset.present? && github_link = link_to_github(@rubygem) %> | ||
<%= render partial: "rubygems/github_button", locals: { github_link: github_link } %> | ||
|
||
<% if github_params = github_params(@rubygem) %> |
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.
Does this "shadow" github_params
variable?
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.
I am not sure what do you mean by shadow. I want to render following partial if github_params(@rubygem)
doesn't return nil. used assignment to avoid calling same again in locals
.
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.
github_params
is method, but also becomes variable in here after assignment. This emits warning in pure Ruby if I remember well. Maybe not problem for erb.
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.
ah, yes. It does work for erb. changed var name anyway.
d17baed
to
6238da4
Compare
f2d6b88
to
bd619b1
Compare
assert page.has_selector?(".github-btn") | ||
end | ||
|
||
test "shows github link when homepage_uri is set" do |
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.
Is it worth adding a test to validate that there is no Github button when source_code_uri
and homepage_uri
is not a URL to github.com?
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.
Would you say this tests the same thing? let me know if you think this needs integration test as well.
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.
added integration test as well.
40060b5
to
efd62f5
Compare
We had deprecated Linkset a while back, must have missed updating these functions. Links fallbacks to Linkset. Fixes: github button not shown when homepage_uri or source_code_uri is set.
fixes: NoMethodError: undefined method `metadata_uri_set?' for nil:NilClass app/models/links.rb:66:in `block (2 levels) in <class:Links>'
efd62f5
to
6083b9e
Compare
We had deprecated Linkset a while back, must have missed updating
these functions. Links fallbacks to Linkset.
Fixes: github button not shown when homepage_uri or source_code_uri
is set.
closes: #2314