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

Fix inconsistent newline usage %D #1732

Merged
merged 1 commit into from Mar 12, 2018
Merged

Fix inconsistent newline usage %D #1732

merged 1 commit into from Mar 12, 2018

Conversation

hexpunk
Copy link

@hexpunk hexpunk commented Mar 11, 2018

  • Fix inconsistent newline usage %D
  • Update affected unit tests
  • Add tests for spy.printf %D text replacement

Purpose (TL;DR) - mandatory

See #1717 for the detailed bug report. This PR should resolve #1717.

How to verify - mandatory

  1. Check out this branch
  2. npm install
  3. Make sure that %D always returns \n as its first character.
const Sinon = require('sinon');
const spy = Sinon.spy();
spy(1, 2, 3);
console.log(spy.printf('%D').charCodeAt(0));  // => 10
spy(4, 5, 6);
console.log(spy.printf('%D').charCodeAt(0));  // => 10

Or just run npm test, which should pass.

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached.

(That second checklist item isn't really applicable to these changes.)

@mroderick
Copy link
Member

Thank you for the PR @jayandcatchfire.

I don't think you need to update package-lock.json for the changes made in the other files. Do you agree?

@hexpunk
Copy link
Author

hexpunk commented Mar 11, 2018

The changes to the lock file were only a consequence of running npm install. But if you'd rather the lock file not be updated as part of this PR, I'll revert it. Give me a few hours and I'll be able to get in front of a computer and make that change.

Update affected unit tests
Add tests for spy.printf %D text replacement
@hexpunk
Copy link
Author

hexpunk commented Mar 11, 2018

@mroderick The lock file has been reverted.

@mroderick mroderick added the semver:patch changes will cause a new patch version label Mar 12, 2018
@mroderick mroderick merged commit f01d847 into sinonjs:master Mar 12, 2018
@mroderick
Copy link
Member

This has been published as sinon@4.4.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch changes will cause a new patch version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent newlines in %D replacement using spy.printf
2 participants