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

docs: fix ng lint and tools lint command in documentation #7387

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

tmair
Copy link
Contributor

@tmair tmair commented Nov 27, 2023

Description:
This PR tries to fix the ng lint and the tools lint command of the documentation site (see also #7364 (review)).
It is quite a substantial change, as introducing eslint into the build requires to change also changes in some of the source or tests files.

Related issue (if exists):
This is trying to address the request for fixing ng lint sufaced in PR #7364 (review)

@tmair tmair force-pushed the fix-docs-lint-command branch 3 times, most recently from d5ff1e5 to 64a7386 Compare November 27, 2023 14:55
@benlesh
Copy link
Member

benlesh commented Nov 28, 2023

@jakovljevic-mladen or @niklas-wortmann can you please take a look at this one?

@benlesh
Copy link
Member

benlesh commented Dec 24, 2023

ping @jakovljevic-mladen or @niklas-wortmann

@niklas-wortmann
Copy link
Member

Hey sorry for the delay, I am going to have a look at this after the holidays. Happy holidays everyone

Copy link
Member

@jakovljevic-mladen jakovljevic-mladen left a comment

Choose a reason for hiding this comment

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

Hey @tmair, thanks a lot for this PR.

I have left couple of comments that we could address, most of them being related to turning off no-non-null-assertion, but also some other small issues that we could fix.

apps/rxjs.dev/angular.json Show resolved Hide resolved
apps/rxjs.dev/src/app/app.component.ts Outdated Show resolved Hide resolved
apps/rxjs.dev/src/app/shared/toc.service.ts Show resolved Hide resolved
@tmair
Copy link
Contributor Author

tmair commented Jan 3, 2024

I rebased the PR against master and addressed the review comments

@tmair
Copy link
Contributor Author

tmair commented Jan 4, 2024

One more question: Should we add the lint command to the build pipeline or should that be a separate PR once this PR is merged?

Copy link
Member

@jakovljevic-mladen jakovljevic-mladen left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot again @tmair.

@jakovljevic-mladen
Copy link
Member

Should we add the lint command to the build pipeline or should that be a separate PR once this PR is merged?

You can do that in the next PR.

@jakovljevic-mladen jakovljevic-mladen merged commit 10e151e into ReactiveX:master Jan 11, 2024
3 checks passed
@tmair tmair deleted the fix-docs-lint-command branch April 27, 2024 19:25
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.

None yet

5 participants