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

Document spying on accessors #1976

Merged
merged 2 commits into from Mar 4, 2019

Conversation

salomvary
Copy link
Contributor

Closes #1606

Purpose (TL;DR) - mandatory

Adds documentation for an undocumented feature.

How to verify - mandatory

  1. Check out this branch
  2. Open docs/_releases/latest/spies.md

Checklist for author

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

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2790

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.526%

Totals Coverage Status
Change from base Build 2788: 0.0%
Covered Lines: 1664
Relevant Lines: 1730

💛 - Coveralls

@coveralls
Copy link

coveralls commented Feb 11, 2019

Pull Request Test Coverage Report for Build 2817

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 15 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.4%) to 94.15%

Files with Coverage Reduction New Missed Lines %
sinon/mock.js 1 96.62%
sinon/spy.js 2 89.98%
sinon/stub.js 3 91.02%
sinon/util/core/wrap-method.js 9 79.41%
Totals Coverage Status
Change from base Build 2788: -0.4%
Covered Lines: 1650
Relevant Lines: 1718

💛 - Coveralls

@mroderick
Copy link
Member

Documentation updates are very valuable to open source projects ⭐️

The documentation for Sinon is a bit special, so you'll have to also put the changes into all the previous releases for which this applies.

It's sort of hinted in https://github.com/sinonjs/sinon/blob/master/docs/CONTRIBUTING.md#example-documenting-a-fixed-bug, just not in a very obvious way.

Documentation is an area I'd like to significantly improve in 2019.

The feature was added with #692, which became part of sinon@1.14.0, so you'll have to add the changes to all the releases in docs/_releases.

If you can manage to do the changes before March 9th, then we can merge it before it becomes four years after the feature was added 😊

Thank you for contributing to documentation of Sinon 💯

@salomvary
Copy link
Contributor Author

@mroderick Challenge accepted! :)

...for all past releases
@salomvary
Copy link
Contributor Author

For the record, the trick was:

git diff HEAD~ HEAD > doc.diff
find . -path '*docs/_releases/v*' -name spies.md | xargs -I {} patch {} doc.diff

Copy link
Member

@mantoni mantoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! This was pending for a long time. Thank you very much ❤️

@fatso83
Copy link
Contributor

fatso83 commented Mar 4, 2019

I am working on trying to make docs fixes a bit easier "for the rest of us", @mroderick. I'll have something soon.

@mroderick
Copy link
Member

I am working on trying to make docs fixes a bit easier "for the rest of us", @mroderick. I'll have something soon.

Let's talk about that during our call tomorrow ... one of the topics I'd like to talk about is documentation :)

@mroderick mroderick merged commit 3275d18 into sinonjs:master Mar 4, 2019
@mroderick
Copy link
Member

Thank you :)

@fatso83 fatso83 added Property accessors Property Getters/Setters Documentation labels Jun 27, 2019
franck-romano pushed a commit to franck-romano/sinon that referenced this pull request Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Property accessors Property Getters/Setters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for Getter/Setter spying and stubbing
5 participants