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

Fix link for downloading a binary #6732

Merged
merged 1 commit into from
Jan 10, 2019
Merged

Conversation

bmwiedemann
Copy link
Member

Without this patch,
https://build.opensuse.org/package/binary/home:bmwiedemann/git-deps/openSUSE_Leap_15.0/x86_64/git-deps-1-lp150.5.1.noarch.rpm
had

<h3>
Detailed Information About git-deps-1-lp150.5.1.noarch.rpm
<a title="Download file" href="<a href=&quot;https://api.opensuse.org:443/build/home:bmwiedemann/openSUSE_Leap_15.0/x86_64/git-deps/git-deps-1-lp150.5.1.noarch.rpm&quot;>git-deps-1-lp150.5.1.noarch.rpm</a>"><i class='fas fa-download text-secondary'></i>
</a></h3>

@vpereira
Copy link
Contributor

vpereira commented Jan 9, 2019

nice catch @bmwiedemann !

@dmarcoux dmarcoux added Frontend Things related to the OBS RoR app Bootstrap 🚀 Bootstrap migration labels Jan 9, 2019
@bmwiedemann
Copy link
Member Author

btw: I did not actually test the new version

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

thanks @bmwiedemann 💐

Copy link
Contributor

@dmarcoux dmarcoux left a comment

Choose a reason for hiding this comment

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

This produces an error:

nomethoderror

@bmwiedemann
Copy link
Member Author

@dmarcoux https://github.com/openSUSE/open-build-service/blob/master/src/api/app/helpers/webui/webui_helper.rb#L275 has similar syntax, except for the do - what is that intended to do?

@dmarcoux
Copy link
Contributor

dmarcoux commented Jan 9, 2019

The block (so everything under the do) is the content of the link, so what is displayed to the user. When there is a block, link_to's first parameter is the URL of the link. When there isn't a block, the first parameter is the content of the link (for details)

In this case, we don't need the filename. I will push the changes. We already have the download icon and the title, so it's pretty clear with what we use throughout the Bootstrap/new UI.

@dmarcoux
Copy link
Contributor

dmarcoux commented Jan 9, 2019

@vpereira @Ana06 Please review again and feel free to dismiss my review once this is done (as it was prior to the changes I pushed)

@codecov
Copy link

codecov bot commented Jan 9, 2019

Codecov Report

Merging #6732 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6732      +/-   ##
==========================================
+ Coverage   90.68%   90.68%   +<.01%     
==========================================
  Files         473      473              
  Lines       20474    20477       +3     
==========================================
+ Hits        18566    18569       +3     
  Misses       1908     1908

@openSUSE openSUSE deleted a comment from codecov bot Jan 9, 2019
Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

ouch! 🙈

@Ana06 Ana06 dismissed dmarcoux’s stale review January 10, 2019 09:57

he push changes already :)

@Ana06 Ana06 merged commit bc680fc into openSUSE:master Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bootstrap 🚀 Bootstrap migration Frontend Things related to the OBS RoR app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants