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

feat(eslint-config-fluid): Add how to add a new eslint rule in eslint-plugin-fluid package. #23004

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

Conversation

jikim-msft
Copy link
Contributor

Description

22739

This PR adds step by step instructions for:

  • Adding a new rule to the estlint-plugin-fluid
  • Bumping the eslint-plugin-fluid version
  • Bumping the eslint-config-fluid version

@github-actions github-actions bot added the area: build Build related issues label Nov 6, 2024
@github-actions github-actions bot added the base: main PRs targeted against main branch label Nov 6, 2024
Copy link
Contributor

github-actions bot commented Nov 6, 2024

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

linkcheck output


> [email protected] ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start 1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://127.0.0.1:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"


> [email protected] ci:start
> http-server ./public --port 1313 --silent


> [email protected] linkcheck:full
> npm run linkcheck:fast -- --external


> [email protected] linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

http://localhost:1313/docs/start/tree-start/
- (3430:89) 'here' => https://github.com/microsoft/FluidFramework/blob/main/packages/dds/tree/docs/main/merge-semantics.md (HTTP 404)


Stats:
  447834 links
    3441 destination URLs
       2 URLs ignored
       0 warnings
       1 errors

 ELIFECYCLE  Command failed with exit code 1.


Directory structure:

```plaintext
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to close this code block before point 3 below. Also, I think all of it and "DIrectory structure" above don't need the extra indentation.


Once PR 1 is merged, publish the new version of `@fluid-internal/eslint-plugin-fluid` by following the steps:

1. **Publish**: Follow the [guide](https://eng.ms/docs/experiences-devices/opg/office-shared/fluid-framework/fluid-framework-internal/fluid-framework/docs/on-call/release/release) to publish the new version of `@fluid-internal/eslint-plugin-fluid`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just point to "the guidance in our internal documentation" but avoid the link. We probably shouldn't link directly to internal documentation from the public repo.


### 4. Add New Rule to the Appropriate Config

Depending on the scope of the rule, add it to one of the following configurations (NOTE: `recommended` extends thre `minimal-deprecated.js`):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Depending on the scope of the rule, add it to one of the following configurations (NOTE: `recommended` extends thre `minimal-deprecated.js`):
Depending on the scope of the rule, add it to one of the following configurations (NOTE: `recommended` extends `minimal-deprecated.js`):

```json
{
"pnpmOverrides": {
"@fluidframework/eslint-config-fluid": "file:<relative-path-to-config-directory>"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"@fluidframework/eslint-config-fluid": "file:<relative-path-to-config-directory>"
"@fluidframework/eslint-config-fluid": "file:<relative-path-to-eslint-config-fluid-package>"


### 5. Publish New Version of `eslint-config-fluid`

Once the PR is merged, publish the new version of `eslint-config-fluid` following the [guide](https://eng.ms/docs/experiences-devices/opg/office-shared/fluid-framework/fluid-framework-internal/fluid-framework/docs/on-call/release/release).
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above regarding the link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants