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

Updated packages #60

Merged
merged 1 commit into from
Mar 1, 2015
Merged

Updated packages #60

merged 1 commit into from
Mar 1, 2015

Conversation

kahwee
Copy link
Contributor

@kahwee kahwee commented Feb 21, 2015

Hey, not sure if you're okay with this. I think running tests on browserify ~v9.0.0 makes more sense since that is the most likely version people would be using in the near future.

@maxnordlund
Copy link
Collaborator

Don't know about the dependency change, but please change the commit message. That style is fine for pull requests, but not something that will be remembered until the end of time. Commit message styles has been discussed on Stackoverflow and it references a blog post that gives a sane default.


If you didn't know already, here's how to do it:
When you want to change the last commits message, just run git commit --amend and it will drop you into an editor with the current message and some help text. Once your done just run git push --force to overwrite the old commit message with the new one.

@sukima
Copy link
Collaborator

sukima commented Feb 23, 2015

I found these resources very helpful:

The best take away was the because clause. Here is my guidelines (rule of thumbs) that I abide by:

  • Subject line is 50 character or less
  • Subject starts with a command in present tense: "Fix", "Update", "Refactor", "Move", etc. and not "Fixes" or "Fixed".
  • Body is wrapped at 72 characters but don't wrap URL's.
  • Body is in Markdown syntax.
  • Never use git commit -m always on open in an editor. This forces you to think about the message an prevents rambling.
  • When in doubt add the word "because" to the body and go from there.
  • If the code change doesn't warrant a commit message that follows the above then it should probably be rebased differently.

@kahwee
Copy link
Contributor Author

kahwee commented Feb 23, 2015

No problem, I'll take a look and update the commit messages 12 hours later. Thanks for the links.

@eugeneware
Copy link
Owner

Great resources @sukima !

This lets travis to test on a newer version on browserify. Additionally the following packages have been updated as well:

* esprima to ^2.0.0: switched to jquery foundation's initial supported version
* ordered-ast-traverse to "^1.1.1": Changed to get out of 0.x
* mocha to ^2.1.0: clearer test reporter
@kahwee
Copy link
Contributor Author

kahwee commented Feb 28, 2015

I updated the commit messages. Thanks all.

@sukima
Copy link
Collaborator

sukima commented Mar 1, 2015

Awesome! @maxnordlund what do you think?

@maxnordlund
Copy link
Collaborator

It's very much better, I'm happy as it is right now. What about the actual change? Ping @sukima and @eugeneware

@sukima
Copy link
Collaborator

sukima commented Mar 1, 2015

I don't know of the changes between the change in versions to the dependencies. I don't anticipate any breaking changes.

I ran the latest tests on this branch and the four tests pass. My take is the only way to be sure it is all good is to release it to the wild. So I'll merge and it can be reverted if others find problems.

sukima added a commit that referenced this pull request Mar 1, 2015
@sukima sukima merged commit a991d00 into eugeneware:master Mar 1, 2015
@maxnordlund
Copy link
Collaborator

Good, that seams reasonable. Thanks, @eugeneware can you add @kahwee to the repo as per #53

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