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

Wrong AppVeyor badge #693

Closed
6 tasks
dtolnay opened this issue Apr 29, 2017 · 5 comments
Closed
6 tasks

Wrong AppVeyor badge #693

dtolnay opened this issue Apr 29, 2017 · 5 comments
Labels
C-bug 🐞 Category: unintended, undesired behavior E-easy E-has-mentor

Comments

@dtolnay
Copy link
Member

dtolnay commented Apr 29, 2017

https://crates.io/crates/serde is showing a badge from:

https://ci.appveyor.com/api/projects/status/github/serde-rs/serde?svg=true&branch=master

The link is correct and shows that the build is passing, but the badge says "unknown".

AppVeyor tells me to use the following badge:

https://ci.appveyor.com/api/projects/status/avsuce1irt8h63u2/branch/master?svg=true

Unclear how this is generated but it would be nice if I could have crates.io show the correct badge.

Implementation steps added by @carols10cents:

  • Add an id field with type Option<String> to the Appveyor badge enum variant
  • Add id: alias('badge.attributes.id') to the appveyor badge component
  • Add an image-url attribute, computed from the id to the appveyor badge component that checks to see if id is defined; if it is, return https://ci.appveyor.com/api/projects/status/{{ id }}/branch/{{ branch }}?svg=true. If not, return https://ci.appveyor.com/api/projects/status/{{ service }}/{{ repository }}?svg=true&branch={{ branch }} (that's what the appveyor badge template currently uses for the image src)
    • I've used handlebars interpolation here but you should use js interpolation, I'm being lazy sorry sorry!
  • Change the appveyor badge template to use the image-url attribute for the img src instead
  • Publish a crate to your local instance that has the URLs dtolnay has listed for serde and manually verify the badge is displayed correctly (we used to have frontend tests for badge tests but they got deleted?)
  • Send a pull request to cargo to update the badge documentation to indicate you can specify the appveyor project id if you want to use that instead
@carols10cents
Copy link
Member

carols10cents commented May 2, 2017

@dtolnay
Copy link
Member Author

dtolnay commented May 2, 2017

You can't prevent someone from creating a duplicate project.

Feature request: given that the inferred badge URL is unreliable, I would like to be able to set the project id and get a reliable badge.

[badges]
appveyor = { repository = "serde-rs/serde", id = "avsuce1irt8h63u2" }

@carols10cents
Copy link
Member

:( i suppose we'll have to do that :(

@carols10cents carols10cents added the C-bug 🐞 Category: unintended, undesired behavior label Aug 2, 2017
mattgathu added a commit to mattgathu/crates.io that referenced this issue Sep 13, 2017
this is part of fixing issue rust-lang#693

✅ Add an id field with type Option<String> to the Appveyor badge enum variant

✅ Add id: alias('badge.attributes.id') to the appveyor badge component

✅ Add an image-url attribute, computed from the id to the appveyor badge component that checks to see if id is defined. (I've used `imageUrl` as attribute)

✅ Change the appveyor badge template to use the image-url attribute for the img src instead

✅ Publish a crate to your local instance that has the URLs dtolnay has listed for serde and manually verify the badge is displayed correctly.
mattgathu added a commit to mattgathu/cargo that referenced this issue Sep 13, 2017
* indicate you can specify the appveyor project id if you want to use that instead

This PR is part of: rust-lang/crates.io#693
@mattgathu
Copy link
Contributor

Hi @dtolnay @carols10cents I have made a PR that addresses this issue. 🙂

bors added a commit to rust-lang/cargo that referenced this issue Sep 13, 2017
…chton

Update Appveyor badge docs

* indicate you can specify the appveyor project id if you want to use that instead

This PR is part of: rust-lang/crates.io#693
bors-voyager bot added a commit that referenced this issue Sep 23, 2017
1050: Fix Wrong AppVeyor badge r=carols10cents

this is part of fixing issue #693

- [x] Add an id field with type Option<String> to the Appveyor badge enum variant

- [x]  Add id: alias('badge.attributes.id') to the appveyor badge component

- [x]  Add an image-url attribute, computed from the id to the appveyor badge component that checks to see if id is defined. (I've used `imageUrl` as attribute)

- [x]  Change the appveyor badge template to use the image-url attribute for the img src instead

- [x]  Publish a crate to your local instance that has the URLs dtolnay has listed for serde and manually verify the badge is displayed correctly.

- [x]  Send a pull request to cargo to update the badge documentation to indicate you can specify the appveyor project id if you want to use that instead. (_https://github.com/rust-lang/cargo/pull/4489_)
@seanprashad
Copy link

@dtolnay @carols10cents Looks like this was supposed to be closed with #1050!

@dtolnay dtolnay closed this as completed Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug 🐞 Category: unintended, undesired behavior E-easy E-has-mentor
Projects
None yet
Development

No branches or pull requests

4 participants