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

have the analyze portion of publishing validation only analyze bin/ and lib/ #4068

Merged
merged 2 commits into from
Dec 4, 2023

Conversation

devoncarew
Copy link
Member

  • have the analyze portion of publishing validation only analyze bin/ and lib/
  • separately, remove lints that are duplicative of package:lints/recommended.yaml

Having the validation step analyze code can be pretty problematic with publishing automation. This preserves the validation static analysis, but scopes it to the portion of a package that's consumed by clients - lib/ and bin/ (removing test/).


  • 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.

@sigurdm sigurdm merged commit db003f2 into master Dec 4, 2023
23 checks passed
@sigurdm sigurdm deleted the skip_analysis_tests branch December 4, 2023 11:46
@devoncarew
Copy link
Member Author

Thanks!

@dcharkes
Copy link
Contributor

dcharkes commented Dec 5, 2023

Drive by comment. So build.dart toplevel script isn't analyzed currently.

@sigurdm
Copy link
Contributor

sigurdm commented Dec 5, 2023

Drive by comment. So build.dart toplevel script isn't analyzed currently.

Right, should it be?

@dcharkes
Copy link
Contributor

dcharkes commented Dec 5, 2023

Drive by comment. So build.dart toplevel script isn't analyzed currently.

Right, should it be?

It's being invoked from other packages to build native assets. So unlike tools/ and test/ it's code used when a package is used as a dependency.

(And so will link.dart be once it's introduced.)

Do we maintain a definitive list of files/directories that is part of the "used as a dependency" somewhere that I can add these things? (Having the list just in lib/src/validator/analyze.dart seems a bit specific, and there will likely be more than one copy in that case?)

@sigurdm
Copy link
Contributor

sigurdm commented Dec 5, 2023

Do we maintain a definitive list of files/directories that is part of the "used as a dependency"

No I don't think we do, the closest is the validator/analyze.dart file that defines what files we analyze on publication.

@dcharkes dcharkes mentioned this pull request Dec 5, 2023
1 task
auto-submit bot pushed a commit that referenced this pull request Dec 8, 2023
`build.dart` (and later `link.dart`) are used by dependencies, so they are part of the set of files of a package which dependencies use (besides just `lib/` and `bin/`).

For more info about `build.dart` and `link.dart` see https://github.com/dart-lang/native.

Follow up of: #4068

---

<details>
  <summary>Contribution guidelines:</summary><br>

- See our [contributor guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md) for general expectations for PRs.
- Larger or significant changes should be discussed in an issue before creating a PR.
- Contributions to our repos should follow the [Dart style guide](https://dart.dev/guides/language/effective-dart) and use `dart format`.
- Most changes should add an entry to the changelog and may need to [rev the pubspec package version](https://github.com/dart-lang/sdk/wiki/External-Package-Maintenance#making-a-change).
- Changes to packages require [corresponding tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing).

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.
</details>
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.

3 participants