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

chore Doc refactor with Admonitions #12498

Closed
wants to merge 0 commits into from
Closed

chore Doc refactor with Admonitions #12498

wants to merge 0 commits into from

Conversation

Biki-das
Copy link
Contributor

this PR is gonna take time , in ref to #12495

made changes to first few docs as proof of concept

@Biki-das
Copy link
Contributor Author

@SimenB this seems to be good change for the doc, i shall for now add :::note and other admonitions which seems appropriate to their places, later we can go through and do a final check .

@weidehai
Copy link
Contributor

@SimenB this seems to be good change for the doc, i shall for now add :::note and other admonitions which seems appropriate to their places, later we can go through and do a final check .

i also make a pr to refactor docs with admonitions,hope it helps

@Biki-das
Copy link
Contributor Author

these needs some tuning regarding the type like which one is note ,which one is a caution ,

@@ -433,11 +431,11 @@ Default: `undefined`

This option allows the use of a custom global setup module, which must export a function (it can be sync or async). The function will be triggered once before all test suites and it will receive two arguments: Jest's [`globalConfig`](https://github.com/facebook/jest/blob/main/packages/jest-types/src/Config.ts#L282) and [`projectConfig`](https://github.com/facebook/jest/blob/main/packages/jest-types/src/Config.ts#L347).

_Note: A global setup module configured in a project (using multi-project runner) will be triggered only when you run at least one test from this project._
:::note A global setup module configured in a project (using multi-project runner) will be triggered only when you run at least one test from this project.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SimenB i think we should wrap all these Notes: into a single :::note ?

docs/CLI.md Outdated
@@ -100,7 +100,7 @@ jest --update-snapshot --detectOpenHandles

## Options

_Note: CLI options take precedence over values from the [Configuration](Configuration.md)._
:::note CLI options take precedence over values from the [Configuration](Configuration.md). :::
Copy link
Contributor

Choose a reason for hiding this comment

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

@Biki-das
Copy link
Contributor Author

Biki-das commented Feb 26, 2022

The final task remaining is distinguishing where to apply tip, caution, danger and info

@Biki-das
Copy link
Contributor Author

@SimenB thoughts on this one?

@mrazauskas
Copy link
Contributor

docs/CLI.md Outdated

:::caution

the cache should only be disabled if you are experiencing caching related problems. On average, disabling the cache makes Jest at least two times slower.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure that after refactoring lines begin with capital letter:

Suggested change
the cache should only be disabled if you are experiencing caching related problems. On average, disabling the cache makes Jest at least two times slower.
The cache should only be disabled if you are experiencing caching related problems. On average, disabling the cache makes Jest at least two times slower.

Comment on lines 151 to 154
:::caution

```javascript
// Note: this will fail
// this will fail
Copy link
Contributor

Choose a reason for hiding this comment

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

Here it is enough to change the comment. Caution block is redundant.

Comment on lines 70 to 74
:::note

In TypeScript, when using `@typess/jest` for example, you can declare the new `toBeWithinRange` matcher in the imported module like this:

:::
Copy link
Contributor

Choose a reason for hiding this comment

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

The code example bellow is part of this Note.

Comment on lines 22 to 28
Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait before aborting.

:::note

The default timeout is 5 seconds.

:::
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here and in similar cases below there is no need for Note. Sounds fine a sentence.

Suggested change
Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait before aborting.
:::note
The default timeout is 5 seconds.
:::
Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait before aborting. The default timeout is 5 seconds.

Comment on lines 12 to 16
:::note

This will only work in Node.js 8+.

:::
Copy link
Contributor

Choose a reason for hiding this comment

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

This one could be mark as History instead of Note (; Just remove it.

docs/Webpack.md Outdated
> Note: Jest caches files to speed up test execution. If you updated .babelrc and Jest is still not working, try running Jest with `--no-cache`.
:::note

Jest caches files to speed up test execution. If you updated .babelrc and Jest is still not working, try running Jest with `--no-cache`.
Copy link
Contributor

Choose a reason for hiding this comment

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

--clearCache sounds like better idea, or?

Suggested change
Jest caches files to speed up test execution. If you updated .babelrc and Jest is still not working, try running Jest with `--no-cache`.
Jest caches files to speed up test execution. If you updated `.babelrc` and Jest is still not working, try running `jest --clearCache`.

@mrazauskas
Copy link
Contributor

Thanks for taking care of this. Many users will benefit, but also this is lots of work. I just went through and left few comments / suggestions. Hope these help.

@Biki-das
Copy link
Contributor Author

Thanks for taking care of this. Many users will benefit, but also this is lots of work. I just went through and left few comments / suggestions. Hope these help.

thank you for the review. I will go through them and apply them.

@Biki-das
Copy link
Contributor Author

@mrazauskas @SimenB i mess up a lot when merge conflicts happen , my last pr got a huge mess and i accidently closed that too and this is bad, can anyone just help me out?

@Biki-das
Copy link
Contributor Author

the lint fail happen when i resolved the conflict now how do i run prettier , because this commit wont show in my git log?

@SimenB
Copy link
Member

SimenB commented Feb 28, 2022

yarn lint:prettier

docs/CLI.md Outdated

:::caution

clearing the cache will reduce performance.
Copy link
Member

Choose a reason for hiding this comment

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

These are sentences, so should be capitalized

docs/CLI.md Outdated
Force Jest to exit after all tests have completed running. This is useful when resources set up by test code cannot be adequately cleaned up. _Note: This feature is an escape-hatch. If Jest doesn't exit at the end of a test run, it means external resources are still being held on to or timers are still pending in your code. It is advised to tear down external resources after each test to make sure Jest can shut down cleanly. You can use `--detectOpenHandles` to help track it down._
Force Jest to exit after all tests have completed running. This is useful when resources set up by test code cannot be adequately cleaned up.

:::tip
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:::tip
:::caution

_Note: Node modules are automatically mocked when you have a manual mock in place (e.g.: `__mocks__/lodash.js`). More info [here](manual-mocks#mocking-node-modules)._
:::note

Node modules are automatically mocked when you have a manual mock in place (e.g.: `__mocks__/lodash.js`). More info [here](manual-mocks#mocking-node-modules).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Node modules are automatically mocked when you have a manual mock in place (e.g.: `__mocks__/lodash.js`). More info [here](manual-mocks#mocking-node-modules).
Node modules are automatically mocked when you have a manual mock in place (e.g.: `__mocks__/lodash.js`). More info [here](ManualMocks.md#mocking-node-modules).


_Note: Core modules, like `fs`, are not mocked by default. They can be mocked explicitly, like `jest.mock('fs')`._
Core modules, like `fs`, are not mocked by default. They can be mocked explicitly, like `jest.mock('fs')`.
Copy link
Member

@SimenB SimenB Feb 28, 2022

Choose a reason for hiding this comment

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

Suggested change
Core modules, like `fs`, are not mocked by default. They can be mocked explicitly, like `jest.mock('fs')`.
Built in modules, like `fs`, are not mocked by default. They can be mocked explicitly, like `jest.mock('fs')`.

_Note: Each glob pattern is applied in the order they are specified in the config. (For example `["!**/__tests__/**", "**/*.js"]` will not exclude `__tests__` because the negation is overwritten with the second pattern. In order to make the negated glob work in this example it has to come after `**/*.js`.)_
:::note

Each glob pattern is applied in the order they are specified in the config. (For example `["!**/__tests__/**", "**/*.js"]` will not exclude `__tests__` because the negation is overwritten with the second pattern. In order to make the negated glob work in this example it has to come after `**/*.js`.)
Copy link
Member

Choose a reason for hiding this comment

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

this should be tip, the second (collectCoverage thing) should be a note

@SimenB
Copy link
Member

SimenB commented Feb 28, 2022

@Biki-das I think we should do one PR per file, or this will be huge and hard to properly review

@Biki-das
Copy link
Contributor Author

@SimenB closing this one , as you said seperate doc for each would be better

@Biki-das Biki-das closed this Feb 28, 2022
@Biki-das Biki-das reopened this Mar 1, 2022
@github-actions
Copy link

github-actions bot commented Apr 1, 2022

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 Apr 1, 2022
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.

None yet

5 participants