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

Fix "unintended_html_in_doc_comment" analysis errors #1291

Merged
merged 5 commits into from
Aug 13, 2024
Merged

Fix "unintended_html_in_doc_comment" analysis errors #1291

merged 5 commits into from
Aug 13, 2024

Conversation

brianquinlan
Copy link
Collaborator

@brianquinlan brianquinlan commented Aug 9, 2024 •
edited
Loading

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Anikate-De
Anikate-De suggested changes Aug 9, 2024
View reviewed changes
pkgs/http/lib/src/base_response.dart Outdated Show resolved Hide resolved
pkgs/http_profile/lib/src/utils.dart Outdated Show resolved Hide resolved
pkgs/http_profile/lib/src/utils.dart Outdated Show resolved Hide resolved
@brianquinlan brianquinlan requested a review from natebosch August 9, 2024 18:14
@natebosch
Copy link
Member

@devoncarew This PR highlights a false positive for the lint. It doesn't understand square braces well enough and it looks like it flags [List<int>]. I think the change to `List<int>` is an improvement, but only slight compared to the obvious improvement from broken HTML. I didn't click on any of these cases when I skimmed the internal results, and the rollout is more churn than I expected.

I also forgot about the unpublished packages where they're unlikely to be noticed since we don't have any workflow where they are visible to users. I don't know whether any IDEs try to render the dartdoc to html for display, but that's the only use case I can imagine is affected.

natebosch
natebosch approved these changes Aug 13, 2024
View reviewed changes
Copy link
Member

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

http and http_profile can both be bumped to the next version with -wip.

@devoncarew
Copy link
Member

@devoncarew This PR highlights a false positive for the lint. It doesn't understand square braces well enough and it looks like it flags [List<int>]. I think the change to `List<int>` is an improvement, but only slight compared to the obvious improvement from broken HTML. I didn't click on any of these cases when I skimmed the internal results, and the rollout is more churn than I expected.

Gotcha; can you file an issue in the sdk for this? We have had some other recent false positives (dart-lang/sdk#56450) addressed - now fixed in head.

@natebosch
Copy link
Member

dart-lang/sdk#56465

@brianquinlan
Copy link
Collaborator Author

http and http_profile can both be bumped to the next version with -wip.

Done

@brianquinlan brianquinlan merged commit 4322382 into dart-lang:master Aug 13, 2024
31 of 34 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 19, 2024
@devoncarew
[deps] rev dartdoc, ecosystem, http, test, tools
1bed4a8 
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/ce09815..b444974):
  b4449742  2024-08-14  Sam Rawlins  Add a doc-import into template_data.dart to resolve 'GeneratorBackend.write' (dart-lang/dartdoc#3832)
  7fb1f3f3  2024-08-14  Sam Rawlins  Fix links in sidebar of extension types (dart-lang/dartdoc#3831)
  392e2aa3  2024-08-14  Sam Rawlins  Simplify some Container fields only used for sidebar logic. (dart-lang/dartdoc#3834)
  31e110d6  2024-08-13  Sam Rawlins  Add tests supporting the wildcard feature (dart-lang/dartdoc#3835)

ecosystem (https://github.com/dart-lang/ecosystem/compare/2719d0c..8626bff):
  8626bff  2024-08-16  Jacob MacDonald  More concise formatting of github workflow summary comments (dart-lang/ecosystem#288)
  de7883c  2024-08-14  Moritz  Update PR Health install instructions (dart-lang/ecosystem#286)
  03bf029  2024-08-14  Moritz  Fix linting (dart-lang/ecosystem#287)

http (https://github.com/dart-lang/http/compare/76512c4..b97b8dc):
  b97b8dc  2024-08-16  Anikate De  pkgs/ok_http: OkHttpClientConfiguration and configurable timeouts. (dart-lang/http#1289)
  4322382  2024-08-13  Brian Quinlan  Fix "unintended_html_in_doc_comment" analysis errors (dart-lang/http#1291)

test (https://github.com/dart-lang/test/compare/8be3c94..cd3dbd5):
  cd3dbd51  2024-08-15  Jacob MacDonald  require approval from core-package-admins for anything under /pkgs (dart-lang/test#2268)
  cfc18ee1  2024-08-14  Jacob MacDonald  add health check job (dart-lang/test#2266)
  f3984a72  2024-08-13  Jacob MacDonald  update changelogs per dart-lang/test#2262 (dart-lang/test#2267)

tools (https://github.com/dart-lang/tools/compare/d563c38..5b15f8b):
  5b15f8b  2024-08-14  Nate Bosch  Add examples of calling two algorithms to README (dart-lang/tools#293)
  ece541c  2024-08-14  Nate Bosch  Ignore unintended_html_in_doc_comment (dart-lang/tools#294)

Change-Id: I23d2f9606e4bfefdfa61e0a4c629f3f05f00c996
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381320
Reviewed-by: Nate Bosch <[email protected]>
Commit-Queue: Devon Carew <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Anikate-De Anikate-De Anikate-De approved these changes

@natebosch natebosch natebosch approved these changes

Assignees
No one assigned
Labels
package:http_client_conformance_tests package:http
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@brianquinlan @natebosch @devoncarew @Anikate-De