PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

PR Code Suggestions ✨

Explore these optional code suggestions:

-export default async function toplevel(cwd = process.cwd()) {
-	return await escalade(cwd, (directory, files) => {
+export default function toplevel(cwd = process.cwd()) {
+	return escalade(cwd, (directory, files) => {

-export default async function toplevel(cwd = process.cwd()) {
-	return await escalade(cwd, (directory, files) => {
+export default function toplevel(cwd = process.cwd()) {
+	return escalade(cwd, (directory, files) => {
 		if (files.includes(".git")) {
 			return directory;
 		}
 	});
 }

Great idea! Thanks!

Got this AI feedback:

PR Review Summary

Thanks for this PR! The dependency reduction is a great improvement. However, there are a few items that need to be addressed before merging:

✅ What's Good

• Excellent dependency reduction: Removes 6 packages from the tree (find-up + 5 transitive deps)
• No new dependencies: escalade is already present via yargs
• Smaller bundle size: escalade is only ~183-210 bytes
• Code simplification: Reduces from 28 to 13 lines

❌ Required Changes

1. Remove unnecessary async/await (Code Style)
   The async/await keywords are redundant since you're just passing through the promise:
   -export default async function toplevel(cwd = process.cwd()) {
   • return await escalade(cwd, (directory, files) => {
     +export default function toplevel(cwd = process.cwd()) {
   • return escalade(cwd, (directory, files) => {
     if (files.includes(".git")) {
     return directory;
     }
     });
     }
2. Add Tests (CRITICAL - Currently Missing)
   There are currently no tests for @commitlint/top-level. Before merging, we need tests to verify:
   • ✅ Finding normal git repos (.git as directory)
   • ✅ Finding git submodules (.git as file)
   • ✅ Handling non-git directories (should return undefined)
   • ✅ Respecting the cwd parameter
   • ✅ Working from nested directories
     This is especially important because the original code explicitly handled .git as both a file (submodules) and directory (normal repos). We need to verify escalade handles both cases correctly.
3. Manual Testing Verification
   Please confirm you've tested this change in:
   • A normal git repository
   • A git submodule (where .git is a file, not a directory)
   • A directory that's not in a git repo
   • Using commitlint CLI end-to-end

❓ Question

The original code explicitly checked for .git as both a file (submodules) and directory (normal repos). The escalade callback's files parameter includes both files and directories in its array. Can you confirm this works correctly for submodules where .git is a file pointing to the parent repo's modules directory?

Recommendation

Once tests are added and the async/await is fixed, this PR will be ready to merge. The dependency reduction is valuable and escalade is a solid choice!

After looking at this it might be better to first add tests to the module before we exchange the dep. wdyt?

@escapedcat, what about this?

After looking at this it might be better to first add tests to the module before we exchange the dep. wdyt?

Sorry, the main point here is to first add tests to the current master for top-level, let those pass and then make sure they still pass with this PR. Makes sense?

Thanks! Is it possible to include further test cases?:

Test Coverage Analysis:
The PR author added these 3 tests:

1. ✅ Finding normal git repos (.git as directory)
2. ✅ Finding git submodules (.git as file)
3. ✅ Handling non-git directories (returns undefined)

However, from my original review requirements, these are missing:

• ⚠️ Testing with explicit cwd parameter (partially covered, but not explicitly tested)
• ⚠️ Testing from deeply nested directories (the tests use path.join(temp, "src", "shared") which is only 2 levels deep)

• ⚠️ Testing with explicit cwd parameter (partially covered, but not explicitly tested)

I have do it before.

• ⚠️ Testing from deeply nested directories (the tests use path.join(temp, "src", "shared") which is only 2 levels deep)

I will try this.

Thanks!

Thanks! 👏

Now you can re-add the commits from before. Sorry for the confusion!

❤️