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

Add url to errors #286

Merged
merged 3 commits into from
Mar 24, 2017
Merged

Conversation

alextes
Copy link
Contributor

@alextes alextes commented Mar 23, 2017

Currently, the property is called url. Another sensible choice could be href.


Fixes #282

@sindresorhus
Copy link
Owner

sindresorhus commented Mar 24, 2017

@alextes Add Fixes #666 (Use the actual issue number of course) to PR descriptions so the issue is closed when the PR is merged. More tips here: https://github.com/tiimgreen/github-cheat-sheet

@alextes
Copy link
Contributor Author

alextes commented Mar 24, 2017

@sindresorhus it's in the commit message of the commit that fixes it. I'll use fixes from now on and add it to the initial PR comment instead. Makes more sense to me after thinking about the options.

Hm, the effect of the latter is nicer too I see. It includes a PR status badge. Thanks for the suggestion Sindre!

@sindresorhus
Copy link
Owner

It also makes it easy to quickly click into the issue.

@sindresorhus
Copy link
Owner

@floatdrop Looks good to you?

@floatdrop
Copy link
Contributor

@sindresorhus lgtm.

@sindresorhus
Copy link
Owner

This also needs to be documented: https://github.com/sindresorhus/got#errors Forgot about that.

@alextes
Copy link
Contributor Author

alextes commented Mar 24, 2017

Done. Missed that on my previous PR - the one that added protocol - too.

@sindresorhus sindresorhus merged commit a287f8e into sindresorhus:master Mar 24, 2017
@alextes alextes deleted the add-url-to-errors branch March 24, 2017 09:48
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.

None yet

3 participants