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

Rewrite some read bumps in pretty-format #4093

Merged
merged 3 commits into from
Jul 20, 2017

Conversation

pedrottimark
Copy link
Contributor

Summary

Rewrite some small bumps that have distracted me from understanding the big picture:

  • Restructure the minority of exceptions to no-else-return rule (and else if return structure) to follow same pattern as the majority of conditional returns. Thinking forward to future code changes, the simple if return structure provides a minimal diff like trailing comma.
  • Replace key with name in printMap to be parallel with printObject
  • Replace entries with values iterator in printSet
  • Replace refs.indexOf(val) > -1 with refs.indexOf(val) !== -1 in printComplexValue
  • Move refs.slice() after if return in printComplexValue

To help future readers understand the recently refactored helper functions:

  • Replace printList with printListItems
  • Replace printMap with printMapEntries
  • Replace printObject with printObjectProperties
  • Replace printSet with printSetValues

For your info: goals for next week

  • Provide alternative plugin serialize API which includes indentation, depth, and refs args
  • Convert standard plugins to alternative serialize API
  • Factor out common code in HTMLElement, ReactElement, and ReactTestComponent
  • Improve test coverage of plugins, if needed

Test plan

Jest

@codecov-io
Copy link

Codecov Report

Merging #4093 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4093      +/-   ##
==========================================
- Coverage   60.35%   60.34%   -0.02%     
==========================================
  Files         195      195              
  Lines        6757     6755       -2     
  Branches        6        6              
==========================================
- Hits         4078     4076       -2     
  Misses       2676     2676              
  Partials        3        3
Impacted Files Coverage Δ
packages/pretty-format/src/index.js 98.48% <100%> (-0.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05dd341...8854048. Read the comment docs.

@cpojer cpojer merged commit d2b7c2d into jestjs:master Jul 20, 2017
@cpojer
Copy link
Member

cpojer commented Jul 20, 2017

Good with me! Looking forward to the next set of PRs :) At the end of all the rewrites, it would be great to do some performance testing with the Chrome Devtools on some heavy workloads and see which micro-optimizations we could do and which are the slowest parts of pretty-format that could be optimized :)

@pedrottimark pedrottimark deleted the refactor-pretty-format branch July 21, 2017 15:01
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Rewrite some read bumps in pretty-format

* Replace ternary with conditional or

* Rename recently refactored helper functions
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants