Enforce BSD copyright headers in dart_skills_lint Dart files#161
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces native binary distribution for dart_skills_lint on macOS and Linux, including a new install.sh script, updated documentation, and a test to ensure all Dart files contain the standard BSD copyright header. Feedback focuses on improving cross-platform compatibility, specifically by handling Windows-style line endings (\r\n) in the copyright header test and the checksum verification script, as well as refining the install.sh script to correctly determine if sudo is needed when installing to nested directories.
| needs_sudo=0 | ||
| if [ -d "$INSTALL_DIR" ]; then | ||
| [ -w "$INSTALL_DIR" ] || needs_sudo=1 | ||
| else | ||
| parent="$(dirname "$INSTALL_DIR")" | ||
| [ -d "$parent" ] && [ -w "$parent" ] || needs_sudo=1 | ||
| fi |
There was a problem hiding this comment.
If INSTALL_DIR is a nested directory that does not exist yet (e.g., ~/my-tools/bin), checking only the immediate parent directory dirname "$INSTALL_DIR" will fail if that parent also doesn't exist. This incorrectly sets needs_sudo=1 even if the user has full write permissions to create the nested directory structure under their home directory, leading to unnecessary sudo prompts and root-owned directories in user space. Finding the closest existing parent directory and checking its writability resolves this issue.
| needs_sudo=0 | |
| if [ -d "$INSTALL_DIR" ]; then | |
| [ -w "$INSTALL_DIR" ] || needs_sudo=1 | |
| else | |
| parent="$(dirname "$INSTALL_DIR")" | |
| [ -d "$parent" ] && [ -w "$parent" ] || needs_sudo=1 | |
| fi | |
| needs_sudo=0 | |
| target_dir="$INSTALL_DIR" | |
| while [ ! -d "$target_dir" ] && [ "$target_dir" != "/" ] && [ "$target_dir" != "." ]; do | |
| target_dir="$(dirname "$target_dir")" | |
| done | |
| [ -w "$target_dir" ] || needs_sudo=1 |
Adds test/copyright_header_test.dart which scans every .dart file in bin/, lib/, and test/ and fails if any is missing the BSD copyright block. Bacfills the 15 files that were missing it, closing #160.
reidbaker
force-pushed
the
enforce-copyright-headers
branch
from
da4678a to
aa05580
Compare
| final String packageRoot = p.normalize(p.absolute('.')); | ||
| final List<String> missing = []; | ||
|
|
||
| for (final dir in ['bin', 'lib', 'test']) { |
There was a problem hiding this comment.
Make this a set of directories and document the relative path they are defined from.
| 'The following Dart files are missing the BSD copyright header:\n' | ||
| ' ${missing.join('\n ')}\n\n' | ||
| 'Add the following block as the first three lines of each file:\n' | ||
| ' // Copyright (c) <year>, the Dart project authors. Please see the AUTHORS file\n' |
There was a problem hiding this comment.
Put the year we are using in the exiting headers instead of using so that the copyright is copy pasteable.
There was a problem hiding this comment.
and extract the copyright that is pasted into a constant variable.
- Normalize \r\n -> \n in _hasCopyrightHeader so the test passes on Windows checkouts with core.autocrlf enabled. - Extract the canonical copyright text into a _copyrightHeader constant and use it in the error message (with 2026 instead of <year> so the block is copy-pasteable). - Convert source directories to a const Set<String> with a doc comment naming the path base. - Revert bin/cli.dart's shebang/copyright spacing to match main; the blank line came from a dev-channel formatter and is rejected by CI's stable formatter (and by pana).
2 participants
Summary
test/copyright_header_test.dartwhich scans every.dartfile underbin/,lib/, andtest/and fails if any is missing the three-line BSD copyright block (year is not pinned; shebang files are handled correctly)Closes #160.