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

Removed ability to specify the badge's label for the “website” service #965

Merged
merged 1 commit into from
Apr 26, 2017

Conversation

SuzanneSoy
Copy link
Contributor

@paulmelnikow Following your suggestion in #949 (comment), I removed the part of the regexp concerned with the badge's label, and adjusted the code accordingly.

The new version seems to work fine, I re-ran the tests below (moving the label to a query parameter where applicable).

http://127.0.0.1/website/https/img.shields.io/nonexistant/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|offline (red)
http://127.0.0.1/website/https/img.shields.io/badge/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|online (green)
http://127.0.0.1/website-up-down/https/img.shields.io/nonexistant/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|down (red)
http://127.0.0.1/website-up-down/https/img.shields.io/badge/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|up (green)
http://127.0.0.1/website-up-down-blue-orange/https/img.shields.io/nonexistant/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|down (orange)
http://127.0.0.1/website-up-down-blue-orange/https/img.shields.io/badge/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|up (blue)
http://127.0.0.1/website---u--//p---d--own//-blue-orange/https/img.shields.io/badge/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|-u-/p- (blue)
http://127.0.0.1/website---u--//p---d--own//---blue-orange/https/img.shields.io/nonexistant/foo-bar-green.svg.svg?label=si-t-e/do/-cs
si-t-e/do/-cs|d-own/- (orange)
http://127.0.0.1/website-up-down/https/img.shields.io/website-up-down/https/img.shields.io.svg.svg?label=so-meta
so-meta|up (green)
http://127.0.0.1/website---u--//p-%EF%BB%BF--d--own//---blue-orange/https/img.shields.io/nonexistant/foo-bar-green.svg.svg?label=-si-t-e/do/-cs
-si-t-e/do/-cs|-d-own/- (orange)
http://127.0.0.1/website-online-unavailable-blue-red/https/en.wikipedia.org/wiki/Documentation.svg?label=docs
docs|online (blue)
http://127.0.0.1/website-up-down-blue-orange/http/shields.io.svg
website|up (blue)
http://127.0.0.1/website-up-down-blue-orange/http/shields.io/nonexistant.svg
website|down (orange)
http://127.0.0.1/website-up-down/http/shields.io.svg
website|up (green)
http://127.0.0.1/website-up-down/http/shields.io/nonexistant.svg
website|down (red)
http://127.0.0.1/website/http/shields.io.svg
website|online (green)
http://127.0.0.1/website/http/shields.io/nonexistant.svg
website|offline (red)

@paulmelnikow
Copy link
Member

paulmelnikow commented Apr 26, 2017

Looks great, and the results of my tests look good too.

One thought: want to keep ?label= in the example?

@paulmelnikow paulmelnikow added the service-badge New or updated service badge label Apr 26, 2017
…e, as this is already possible with ?label=some-text
@SuzanneSoy
Copy link
Contributor Author

@paulmelnikow I added the ?label=… to try.html. What scared me is that when index.html is generated, it automatically adds ?maxAge=2592000, and I'm not sure that was designed to handle the case where there already are query parameters. If the ?label=… causes problems when generating index.html, I can remove it again.

@paulmelnikow
Copy link
Member

Ooh, good thought. I just checked by running make website, and it looks like it does the right thing:

<tr><th data-keywords='website' data-doc='websiteDoc'> Website: </th>
  <td><img src='https://img.shields.io/website-up-down-green-red/http/shields.io.svg?label=my-website&maxAge=2592000' alt=''/></td>
  <td><code>https://img.shields.io/website-up-down-green-red/http/shields.io.svg?label=my-website</code></td>
</tr>

@paulmelnikow paulmelnikow merged commit db2aafa into badges:master Apr 26, 2017
@paulmelnikow
Copy link
Member

Thanks for addressing this so quickly!

@paulmelnikow paulmelnikow added this to the Next Deploy milestone Apr 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants