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

Jump to bottom

bad appveyor badges for projects with underscore #587

Closed
goertzenator opened this issue Mar 3, 2017 · 11 comments · Fixed by #1111
Closed

bad appveyor badges for projects with underscore #587

goertzenator opened this issue Mar 3, 2017 · 11 comments · Fixed by #1111
Labels
A-frontend 🐹 C-bug 🐞 Category: unintended, undesired behavior E-easy E-help-wanted

Comments

@goertzenator
Copy link

The appveyor badge on https://crates.io/crates/erlang_nif-sys does not load, however the link does work when clicked.

The problem is that the link to the appveyor project requires underscores converted to dashes, but the link to the badge requires an unmangled name. The current crates.io code appears to use the same name for both.

I currently use a premangled repository name in the badges section:

[badges]
travis-ci = { repository = "goertzenator/erlang_nif-sys" }
appveyor  = { repository = "goertzenator/erlang-nif-sys" }

The resulting broken badge URL
https://ci.appveyor.com/api/projects/status/github/goertzenator/erlang-nif-sys?svg=true&branch=master

The corrected URL is:
https://ci.appveyor.com/api/projects/status/github/goertzenator/erlang_nif-sys?svg=true&branch=master

I expect I could make the badge image to work by changing the badge setting to appveyor = { repository = "goertzenator/erlang_nif-sys" }, but then the link would break. I think both should work.
@Eh2406
Copy link
Contributor

Eh2406 commented Mar 3, 2017 •
edited

Do you know, or have links for, when in general mangling is needed?

I think fixing this will involve adding a repository_mangled field to badge-appveyor.js with js for mangling and modifying badge-appveyor.hbs to accept it. But I am new to the project so there may be better ways to do this.

@goertzenator
Copy link
Author

I'm not aware of any documented mangling.

For fun I just created a new github repo and added it to appveyor, and sure enough the underscore was converted to a dash in the URL.

Could the unmangled repo name be dug out from here?:
https://ci.appveyor.com/api/projects/goertzenator/erlang-nif-sys/settings

<RepositoryName>goertzenator/erlang_nif-sys</RepositoryName>
<RepositoryScm>git</RepositoryScm>
<RepositoryType>gitHub</RepositoryType>

@goertzenator
Copy link
Author

Alternatively, that settings page also contains...

<WebhookId>rssa03e29mxou4hv</WebhookId>

And with that the badge can be fetched more robustly:

https://ci.appveyor.com/api/projects/status/rssa03e29mxou4hv?svg=true

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 3, 2017

When I go to the settings page I get

<Error>
<Message>Authorization required</Message>
</Error>

@carols10cents
Copy link
Member

I think crates.io should expect an unmangled name, so you should specify:

[badges]
appveyor  = { repository = "goertzenator/erlang_nif-sys" }

Then the appveyor component should have another computed property that replaces _ in the badge.attributes.repository with -. I'd probably name the computed property normalizedRepository, just because "mangled" sounds like we're making the repository name unrecognizable or something.

And then badge-appveyor.hbs should use normalizedRepository for the a href.

@goertzenator
Copy link
Author

I agree with that direction @carols10cents , but I'd like to suggest some refinements:

  • normalizedRepository is really just the project name in Appveyor lingo. Consider calling it project or projectname.
  • In the badges section allow the direct specification of project, but the default will be computed from repository. For example...
[badges]
appveyor  = { repository = "goertzenator/erlang_nif-sys", project = "goertzenator/erlang-nif-sys"}

The motivation for this come from this bug in which the Appveyor project name does not follow changes in repository name. Being able to separately specify repository and project would help in this case:

[badges]
appveyor  = { repository = "goertzenator/erlang_nif-sys", project = "goertzenator/totally_new_repo"}

@carols10cents
Copy link
Member

That all sounds good, @goertzenator! Also the documentation (that lives in cargo) should be updated to reflect whatever gets implemented here.

@carols10cents carols10cents added this to the impl period milestone Sep 13, 2017
@carols10cents carols10cents removed this from the impl period milestone Sep 15, 2017
@treiff
Copy link
Contributor

treiff commented Oct 3, 2017

@carols10cents, @goertzenator If no one is working on this, I wouldn't mind giving it a shot.

This was referenced Oct 8, 2017
Merged
Merged
bors added a commit to rust-lang/cargo that referenced this issue Oct 8, 2017
@bors
Auto merge of #4595 - treiff:update-appveyor-badge-documentation, r=a…
cf08602 
…lexcrichton

Update docs for appveyor badge.

Can now optionally specify the appveyor `project_name`.

Related PR: [rust-lang/crates.io#1111](rust-lang/crates.io#1111)
Related Issue: [rust-lang/crates.io#587](rust-lang/crates.io#587)
bors-voyager bot added a commit that referenced this issue Oct 11, 2017
@bors-voyager
Merge #1111
e70ae13 
1111: Fix appveyor badge display and link. r=carols10cents

This fixes an issue with appveyor badges where if your repository name included an underscore, the link generated would not work because appveyor urls use dashes.

You can now specify an optional `project_name` for the appveyor badge in your `Cargo.toml`
```
[badges]
appveyor = { repository = "example/test_crate", service = "github", project_name = "example/test-crate" }
```

If the optional `project_name` attribute is present, we will use that for the link URL, otherwise we default to the required `repository` attribute and replace the underscores with dashes.

The reason to provide the `project_name` option is that if you change the repository name appveyor doesn't track the change.

Let me know if I missed anything!  I did create a test crate and set it up with appveyor to confirm.

Closes #587
@sagiegurari
Copy link

was this fix deployed to crates.io? because it doesn't work for me
https://crates.io/crates/rust_info

@sagiegurari sagiegurari mentioned this issue Oct 13, 2017
Closed
@carols10cents
Copy link
Member

Not yet, sorry! Will deploy right now.

@sagiegurari
Copy link

thanks

@treiff treiff mentioned this issue Oct 13, 2017
Merged
bors added a commit to rust-lang/libc that referenced this issue Oct 14, 2017
@bors
Auto merge of #804 - treiff:update-appveyor-badge, r=alexcrichton
49ffdf6 
Add `project_name` attribute to appveyor badge.

There was a recent [issue](rust-lang/crates.io#587) in crates.io where appveyor is looking for a dash separated URL for the link to appveyors site:
```
https://ci.appveyor.com/project/rust-lang-libs/libc
```
 and the actual repo name for the image badge url:
```
https://ci.appveyor.com/api/projects/status/github/rust-lang/libc?svg=true
```
We recently added the ability to specify a `project_name` that fixes this issue.  You'll notice that currently on https://crates.io/crates/libc the appveyor badge on the right sidebar show `build unknown`, this PR will address that issue.

Please let me know if you have any questions.
@tari tari mentioned this issue Jan 6, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-frontend 🐹 C-bug 🐞 Category: unintended, undesired behavior E-easy E-help-wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix appveyor badge display and link. treiff/crates.io
5 participants
@carols10cents @goertzenator @Eh2406 @treiff @sagiegurari