Skip to content

#1001 Add a comment to fields used by shields.io #1421

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

Merged

Conversation

ericmoritz
Copy link
Contributor

Fix for #1001. I guessed on the formatting of the comment. I can easily change it if needed.

@sgrif
Copy link
Contributor

sgrif commented Jun 6, 2018

I'm not really sure that we need to specifically mark fields used by one client. We shouldn't be making any backwards incompatible changes to our API, as it is versioned and we know folks rely on it. (I'm not sure what specifically happened with #1001, I would guess that it never worked until it was fixed).

However, that issue has been open for a year so I'll defer to @carols10cents

@Turbo87
Copy link
Member

Turbo87 commented Jun 7, 2018

@sgrif I believe #787 is the PR that introduced the problem

@ericmoritz
Copy link
Contributor Author

@sgrif I won't mind if we close this PR. I was bored last night and grabbed a low hanging fruit.

@carols10cents
Copy link
Member

I'm not really sure that we need to specifically mark fields used by one client.

We already mark the routes that cargo uses; this doesn't feel much different.

We shouldn't be making any backwards incompatible changes to our API, as it is versioned and we know folks rely on it.

Mistakes happen, we haven't documented the requests that our frontend uses, and we have been known to say "oh we're the only one using this, we can change it" and then been wrong.

I like this change. Thank you @ericmoritz !!

bors: r+

bors-voyager bot added a commit that referenced this pull request Jun 26, 2018
1421: #1001 Add a comment to fields used by shields.io r=carols10cents

Fix for #1001. I guessed on the formatting of the comment. I can easily change it if needed.
@bors-voyager
Copy link
Contributor

bors-voyager bot commented Jun 26, 2018

Build succeeded

@bors-voyager bors-voyager bot merged commit 3250194 into rust-lang:master Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants