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

accepts hostnames without TLD, and i18n formats (punycode). #1390

Merged
merged 3 commits into from
Jul 14, 2016

Conversation

Tieske
Copy link
Member

@Tieske Tieske commented Jul 13, 2016

fixes #1300 and i18n hostnames. See #1205 (comment)

@Tieske Tieske added this to the 0.9 milestone Jul 13, 2016
@thibaultcha
Copy link
Member

Could we have more descriptive changes in PRs?

  • adds support for "X" characters
  • removes support for "Y" characters
  • update tests for both (or not)
  • "X" character allows for [example hostnames]

I generally try to wrap PRs in 1 commit (when possible) and follow the commit format, with a body that is written as if it was a PR description (or vice-versa)

feat(resolver) allow for X remove Y

- adds support for "X" characters
- removes support for "Y" characters
- update tests for both (or not)
- "X" character allows for [example hostnames]

Fix #1200 and #1205 (comment)

This way, GitHub grabs the commit message directly as the PR's description, and I only have to click the "Open PR" button.

@thibaultcha
Copy link
Member

Example: #1151

Simply explaining the issue and the fix in the commit messages allows for detailed explanation in both the git history for future reference and in the PR for reviewers. Too often I try to understand a PR by reading the changelog and it takes quite longer to understand.

@Tieske Tieske self-assigned this Jul 14, 2016
@thibaultcha thibaultcha added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) and removed pr/status/needs review labels Jul 14, 2016
@Tieske Tieske merged commit b2ff61f into next Jul 14, 2016
@Tieske Tieske deleted the fix/hostname-i18n branch July 14, 2016 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants