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

Remove unnecessary ! #1220

Merged
merged 1 commit into from
Oct 25, 2017
Merged

Remove unnecessary ! #1220

merged 1 commit into from
Oct 25, 2017

Conversation

bappi
Copy link
Contributor

@bappi bappi commented Oct 25, 2017

Changes proposed in this Pull Request:

Related issue(s):

@davidakennedy davidakennedy merged commit ee3d905 into Automattic:master Oct 25, 2017
@davidakennedy
Copy link
Contributor

@B-07 Thanks for the PR. Great catch!

One thing I'll mention: please put an explanation of your changes in the PR. No matter how simple. It's tedious sometimes, yes. However, this is a good habit to learn as a developer because:

  • You get better at explaining things.
  • You make a project maintainer's life easier.
  • It's helpful when digging back through history, wondering why a certain change was made.
  • We kindly ask you to do it by providing a PR template. 😄

Thanks again for your contributions!

@bappi
Copy link
Contributor Author

bappi commented Oct 25, 2017

@davidakennedy Thank for the suggestions, I'll follow those from now on for sure.

davidakennedy added a commit that referenced this pull request Oct 25, 2017
@grappler
Copy link
Collaborator

@davidakennedy I don't think this was supposed to be removed. We added it deliberately in e9c7d73 in #770

@davidakennedy
Copy link
Contributor

Ah, I looked and missed that @grappler! Thanks for catching it. Will revert.

davidakennedy added a commit that referenced this pull request Oct 31, 2017
* This is needed code. See: #770.
* Also, see #1220.
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.

3 participants