Skip to content

Conversation

@araujogui
Copy link
Member

@araujogui araujogui commented Feb 17, 2025

Description

Create linter system

Validation

Run CLI

Related Issues

Fixes #56

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I've covered new added functionality with unit tests if necessary.

flakey5 and others added 4 commits February 17, 2025 12:44
Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
@araujogui araujogui marked this pull request as ready for review February 19, 2025 15:06
@araujogui araujogui requested a review from a team as a code owner February 19, 2025 15:06
Copy link
Member

@flakey5 flakey5 left a comment

Choose a reason for hiding this comment

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

first pass will finish in a bit

@AugustinMauroy
Copy link
Member

do we have ability to warn if it's an unknown stability level ?

@araujogui
Copy link
Member Author

do we have ability to warn if it's an unknown stability level ?

I didn't create this rule but it's a good idea, I can develop this.

@flakey5
Copy link
Member

flakey5 commented Feb 21, 2025

Wdyt of adding a capability here to ignore only specific linting errors? I.e. being able to ignore an invalid introduced_in but still getting errors on everything else

@AugustinMauroy
Copy link
Member

+1 for ingnoring rules

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

Such a clean code! LGTM! ✨

Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

If it's not too complicated have the rules deactivated if not LGTM

Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

LGTM, is there any way to test this?

@AugustinMauroy
Copy link
Member

LGTM, is there any way to test this?

clone this repo npx api-docs-tooling -i "/path/to/node/doc/api/*.md"

@bjohansebas
Copy link
Member

LGTM, is there any way to test this?

clone this repo npx api-docs-tooling -i "/path/to/node/doc/api/*.md"

I was referring to more automated tests without having to take that step you mentioned.

@ovflowd
Copy link
Member

ovflowd commented Mar 2, 2025

Agreed some unit tests would be cool

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

Can we add some unit tests, @araujogui?

@araujogui
Copy link
Member Author

Can we add some unit tests, @araujogui?

Sure, I'll push this tomorrow. Disabling rules too.

Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

LGTM!

@araujogui
Copy link
Member Author

GitHub seems to be automatically stripping ANSI escape sequences, so tests pass locally and fail in CI

@ovflowd
Copy link
Member

ovflowd commented Mar 6, 2025

GitHub seems to be automatically stripping ANSI escape sequences, so tests pass locally and fail in CI

Any workaround?

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM

@araujogui
Copy link
Member Author

araujogui commented Mar 6, 2025

GitHub seems to be automatically stripping ANSI escape sequences, so tests pass locally and fail in CI

Any workaround?

Yep, FORCE_COLOR=1 fixed the issue.

@araujogui araujogui merged commit 79cc21c into main Mar 7, 2025
7 checks passed
@araujogui araujogui deleted the flakey5/56 branch March 7, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Introduce a Linter / Validation System

8 participants