-
Notifications
You must be signed in to change notification settings - Fork 7
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
add invalid_runtime_check_with_js_interop_types, unintended_html_in_doc_comment #285
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthBreaking changes ✔️Details
Changelog Entry ✔️Details
Changes to files need to be accounted for in their respective changelogs. Coverage ✔️Details
This check for test coverage is informational (issues shown here will not fail the PR). API leaks ✔️DetailsThe following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
License Headers ✔️Details
All source files should start with a license header. Package publish validation ✔️Details
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two good lints!
|
||
- Contribute a (brief) change policy to the readme. | ||
- Contributed a (brief) change policy to the readme. | ||
- Added `invalid_runtime_check_with_js_interop_types`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good to add this lint, but I imagine this being in a minor release might break CI for a few repos, as I found quite a few cases of this lint being hit.
That might be a good thing though, as it's almost always indicating an error!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok to add new lints in a minor version for this package; we've made that call before. It should only break our own repos and we can update as we see failures. It also lets us dogfood earlier - otherwise we'd have to send PRs to each repo to rev the version constraint before we saw the new version.
... as I found quite a few cases of this lint being hit
That's good data - we were wondering how much this would trigger on existing code.
Should we add |
I guess you mean |
@parlough – yes, that's what i mean – @devoncarew ? |
Ah the set already has
|
Yup, already added |
Revisions updated by `dart tools/rev_sdk_deps.dart`. ecosystem (https://github.com/dart-lang/ecosystem/compare/f977423..2719d0c): 2719d0c 2024-08-08 Devon Carew add invalid_runtime_check_with_js_interop_types, unintended_html_in_doc_comment (dart-lang/ecosystem#285) http (https://github.com/dart-lang/http/compare/73fce77..76512c4): 76512c4 2024-08-07 Kate test(http_client_conformance_tests): Remove old skips (dart-lang/http#1284) d7ae256 2024-08-08 Anikate De [docs] sort pkg list in ascending order (dart-lang/http#1287) b82d88c 2024-08-06 Anikate De [docs] Add ok_http entry to readme (dart-lang/http#1285) package_config (https://github.com/dart-lang/package_config/compare/f0b7256..76934c2): 76934c2 2024-08-06 Kevin Moore Latest lints, require Dart 3.4 (dart-lang/package_config#157) sync_http (https://github.com/dart-lang/sync_http/compare/ab8377e..91c0dd5): 91c0dd5 2024-08-12 dependabot[bot] Bump actions/checkout from 4.1.6 to 4.1.7 (google/sync_http.dart#49) test (https://github.com/dart-lang/test/compare/9fbbfdb..8be3c94): 8be3c948 2024-08-12 Ömer Sinan Ağacan Run dart2wasm integration test on Windows (dart-lang/test#2265) e656e5a9 2024-08-12 Yaroslav Vorobev fix: use `toFilePath` in package config Uri (dart-lang/test#2262) 6bfe0d62 2024-08-12 Ömer Sinan Ağacan Fix documentation rendering issues (dart-lang/test#2264) tools (https://github.com/dart-lang/tools/compare/55dbd6e..d563c38): d563c38 2024-08-13 Moritz Add health workflow (dart-lang/tools#292) 8ac5509 2024-08-12 Devon Carew Update CODEOWNERS for package:unified_analytics (dart-lang/tools#289) web (https://github.com/dart-lang/web/compare/e89fe49..4996dc2): 4996dc2 2024-08-12 Srujan Gaddam Ignore unintended_html_in_doc_comment (dart-lang/web#278) Change-Id: I808778af5fb9a1f6885ae847614ffb660fcb8662 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380204 Reviewed-by: Konstantin Shcheglov <[email protected]> Auto-Submit: Devon Carew <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
invalid_runtime_check_with_js_interop_types
unintended_html_in_doc_comment
The idea behind this revision is to use the above two lints for a period of time ourselves (these lints are likely going out with the next major revision of
package:lints
- see https://github.com/orgs/dart-lang/projects/73).Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.