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.

Sign up for GitHub

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

Jump to bottom

Migrate from yarn v1 to npm #2462

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Migrate from yarn v1 to npm #2462

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6638600
Migrate from yarn v1 to npm
andreasgerstmayr Oct 2, 2024
16db232
fix argument propagation in coverage task
andreasgerstmayr Oct 4, 2024
1597556
shorten test command
andreasgerstmayr Oct 4, 2024
c350dba
Apply suggestions from code review
andreasgerstmayr Oct 8, 2024
0c13aa5
update docs per review
andreasgerstmayr Oct 8, 2024
627a8fc
run prettier
andreasgerstmayr Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ FROM node:20.17.0
WORKDIR /workspace
COPY . .

RUN yarn install --frozen-lockfile && cd ./packages/jaeger-ui && yarn build
RUN npm ci && cd ./packages/jaeger-ui && npm run build
11 changes: 5 additions & 6 deletions .github/workflows/lint-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ jobs:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
cache: yarn
cache: npm
node-version: '20'
- run: yarn install --frozen-lockfile
- run: npm ci
- name: Run depcheck
run: yarn run depcheck
- run: yarn lint
- run: yarn build
run: npm run depcheck
- run: npm run lint
- run: npm run build
- name: Ensure PR is not on main branch
uses: jaegertracing/jaeger/.github/actions/block-pr-from-main-branch@main

12 changes: 6 additions & 6 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ jobs:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
cache: yarn
cache: npm
node-version: '20'
- run: yarn install --frozen-lockfile
- run: yarn lint
- run: yarn build
id: yarn-build
- run: npm ci
- run: npm run lint
- run: npm run build
id: npm-build

- name: Package artifacts
id: package-artifacts
run: tar -czvf ./assets.tar.gz --strip-components=3 packages/jaeger-ui/build/
if: steps.yarn-build.outcome == 'success'
if: steps.npm-build.outcome == 'success'

- name: Upload artifacts
uses: svenstaro/upload-release-action@1beeb572c19a9242f4361f4cee78f8e0d9aec5df
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/unit-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ jobs:
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
cache: yarn
cache: npm
node-version: '20'
- run: yarn install --frozen-lockfile
- run: yarn coverage
- run: npm ci
- run: npm run coverage
- name: Upload coverage to codecov.io
uses: codecov/codecov-action@e28ff129e5465c2c0dcc6f003fc735cb6ae0c673 # v4.5.0
with:
Expand Down
2 changes: 1 addition & 1 deletion .npmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
engine-strict=true
engine-strict=true
16 changes: 6 additions & 10 deletions BUILD.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# Build Considerations: Project Root

`yarn` is used instead of `npm`.

## `package.json`

### Dependencies (dev and otherwise)
Expand All @@ -20,7 +18,7 @@ In `./packages/plexus`, `typescript` is used to generate type declarations for t

#### `build`

`yarn build` executes the build in each of `./packages/*` sub-packages.
`npm run build` executes the build in each of `./packages/*` sub-packages.

#### `eslint`

Expand All @@ -37,11 +35,9 @@ This is an amalgamation of linting scripts that run to make sure things are all-

#### `prepare`

Runs after the top-level `yarn install`. This ensures `./packages/plexus` builds and is available to `./packages/jaeger-ui`.

#### `prettier-comment`, `prettier`, `prettier-lint`
Runs after the top-level `npm install`. This ensures `./packages/plexus` builds and is available to `./packages/jaeger-ui`.

`prettier-comment` is just an explanation for why the `./node_module/.bin/bin-prettier.js` path is used instead of just `yarn prettier etc`; it's due to an [issue with `yarn`](https://github.com/yarnpkg/yarn/issues/6300).
#### `prettier`, `prettier-lint`

`prettier` formats the code.

Expand All @@ -59,9 +55,9 @@ Runs after the top-level `yarn install`. This ensures `./packages/plexus` builds

Note that `./packages/plexus` does not yet have any tests, as tracked in issue [#340](https://github.com/jaegertracing/jaeger-ui/issues/340).

`./packages/jaeger-ui` uses [Jest](https://jestjs.io/) for testing. It can be useful to directly run tests for that package by running `yarn test` from its directory, rather than the repository root.
`./packages/jaeger-ui` uses [Jest](https://jestjs.io/) for testing. It can be useful to directly run tests for that package by running `npm run test` from its directory, rather than the repository root.
andreasgerstmayr marked this conversation as resolved.
Show resolved Hide resolved

andreasgerstmayr marked this conversation as resolved.
Show resolved Hide resolved
Tests for React components in `./packages/jaeger-ui` make extensive use of Jest's [snapshot testing](https://jestjs.io/docs/28.x/snapshot-testing) functionality. These snapshots can be regenerated by running `yarn test -u` from the package directory to regenerate all snapshots, or `yarn test -u --testNamePattern <test name>` to regenerate snapshots for a subset of tests only.
Tests for React components in `./packages/jaeger-ui` make extensive use of Jest's [snapshot testing](https://jestjs.io/docs/28.x/snapshot-testing) functionality. These snapshots can be regenerated by running `npm test -- -u` from the package directory to regenerate all snapshots, or `npm test -- -u --testNamePattern <test name>` to regenerate snapshots for a subset of tests only.
Copy link
Member

Choose a reason for hiding this comment

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

can we provide an example from the existing tests? It's not clear what <test name> means, e.g. in this test file:

 PASS  src/utils/readJsonFile.test.js
  fileReader.readJsonFile
    ✓ rejects when given an invalid file (2 ms)
    ✓ does not throw when given an invalid file (1 ms)
    ✓ loads JSON data, successfully (8 ms)
    ✓ loads JSON data (OTLP), successfully (2 ms)
    ✓ rejects an OTLP trace (4 ms)
    ✓ rejects malformed JSON (1 ms)
    ✓ loads JSON-per-line data (1 ms)

Is it the actual title/description? Is "pattern" a reference to some regex?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a regex matching the full test name, i.e. test name and all surrounding describe blocks: https://jestjs.io/docs/cli#--testnamepatternregex
I've updated the text and added a link to the docs.


### `husky` . `hooks` . `pre-commit`

Expand All @@ -79,7 +75,7 @@ Holds GitHub Actions workflows used in CI and in release.

CodeCov is integrated into the unit tests workflow to report coverage data from `./packages/jaeger-ui`. When unit tests are added to Plexus, this integration will need to be updated to gather coverage data for Plexus as well.

[`yarn install --frozen-lockfile`](https://yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-frozen-lockfile) ensures installs in CI fail if they would typically mutate the lockfile.
[`npm ci`](https://docs.npmjs.com/cli/v10/commands/npm-ci) ensures installs in CI fail if they would typically mutate the lockfile.

## `tsconfig.json`

Expand Down
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ git config --add alias.c "commit -s"

# Style guide

Use [typescript](https://www.typescriptlang.org/) for new code. Check types via `yarn tsc-lint`.
Use [typescript](https://www.typescriptlang.org/) for new code. Check types via `npm run tsc-lint`.

We use [`prettier`](https://prettier.io/), an "opinionated" code formatter. It can be applied to both JavaScript and CSS source files via `yarn prettier`.
We use [`prettier`](https://prettier.io/), an "opinionated" code formatter. It can be applied to both JavaScript and CSS source files via `npm run prettier`.

Then, most issues will be caught by the linter, which can be applied via `yarn eslint`.
Then, most issues will be caught by the linter, which can be applied via `npm run eslint`.

Finally, we generally adhere to the [Airbnb Style Guide](https://github.com/airbnb/javascript), with exceptions as noted in our `.eslintrc`.

Expand Down
26 changes: 12 additions & 14 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ Stuck somewhere or found a bug? See [Getting in Touch](https://www.jaegertracing
- [nvm (Node Version Manager)](https://github.com/nvm-sh/nvm)
- [Node.JS](https://nodejs.org/en)
- npm package manager
- [yarn package manager](https://yarnpkg.com/)

The app was built with [create-react-app](https://github.com/facebookincubator/create-react-app).

Expand All @@ -44,10 +43,10 @@ Use the recommended Node versions: (defined in [.nvmrc](./.nvmrc) file):
nvm use
```

Install dependencies via `yarn`:
Install dependencies via `npm`:

```
yarn install --frozen-lockfile
npm install
andreasgerstmayr marked this conversation as resolved.
Show resolved Hide resolved
```

Make sure you have the Jaeger Query service running on http://localhost:16686. For example, you can run Jaeger all-in-one Docker image as described in the [documentation][aio-docs].
Expand All @@ -73,20 +72,20 @@ const proxyConfig = {
Start the development server with hot loading:

```
yarn start
npm run start
andreasgerstmayr marked this conversation as resolved.
Show resolved Hide resolved
```

The above command will run a web server on `http://localhost:5173` that will serve the UI assets, with hot reloading support, and it will proxy all API requests to `http://localhost:16686` where Jaeger query should be running.

#### Commands

| Command | Description |
| ------------ | ------------------------------------------------------------------- |
| `yarn start` | Starts development server with hot reloading and api proxy. |
| `yarn test` | Run all the tests |
| `yarn lint` | Lint the project (eslint, prettier, typescript) |
| `yarn fmt` | Apply Prettier source code formatting |
| `yarn build` | Runs production build. Outputs files to `packages/jaeger-ui/build`. |
| Command | Description |
| --------------- | ------------------------------------------------------------------- |
| `npm run start` | Starts development server with hot reloading and api proxy. |
| `npm run test` | Run all the tests |
| `npm run lint` | Lint the project (eslint, prettier, typescript) |
| `npm run fmt` | Apply Prettier source code formatting |
| `npm run build` | Runs production build. Outputs files to `packages/jaeger-ui/build`. |

### Running on Windows OS

Expand All @@ -96,9 +95,8 @@ Here are some steps to follow:

1. Install WSL: https://learn.microsoft.com/en-us/windows/wsl/install
2. Install Node.JS: https://learn.microsoft.com/en-us/windows/dev-environment/javascript/nodejs-on-wsl
3. Install Yarn: https://dev.to/bonstine/installing-yarn-on-wsl-38p2
4. Connect WSL Environment with VSCode: https://learn.microsoft.com/en-us/windows/dev-environment/javascript/nodejs-on-wsl#install-visual-studio-code
5. Use the WSL Terminal inside VSCode and [follow the Jaeger UI installation steps](#running-the-application)
3. Connect WSL Environment with VSCode: https://learn.microsoft.com/en-us/windows/dev-environment/javascript/nodejs-on-wsl#install-visual-studio-code
4. Use the WSL Terminal inside VSCode and [follow the Jaeger UI installation steps](#running-the-application)

## UI Configuration

Expand Down
Loading