Fix autofixes run on a nested directory #1269
Conversation
🦋 Changeset detectedLatest commit: 9d507df The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // The file outside of our `nestedDir` remains uncommitted. | ||
| expect(statuses).toStrictEqual(['absent', '*added', 'unmodified', 'absent']); |
There was a problem hiding this comment.
Does this check out as reasonable design? If you run skuba lint in /packages/package, I assume it would be surprising for a file in another directory to get committed as part of autofixes.
The overarching counterpoint is that running skuba lint on each subdirectory separately is actually problematic for autofixing in general. What we really need is a single autofix commit to be produced per repository, not per subdirectory.
There was a problem hiding this comment.
I think this may be the best incremental fix we can put in place for now, but it ties in to a broader investigation into how we better support monorepos:
- If a monorepo contains only skuba projects: it may be best to run
skuba lintat the repository root - If a monorepo contains a mix of projects including 1 skuba one: this PR works well
- If a monorepo contains a mix of projects including >1 skuba one: this PR helps but you'd still be running X
lints which would try to push X separate autofix commits
There was a problem hiding this comment.
Makes sense. Multiple commits in it's current state also won't work either right now unless we calculate a dynamic message
| @@ -128,7 +128,7 @@ and maps them to GitHub GraphQL [FileChanges]. | |||
| ```typescript | |||
| import { GitHub } from 'skuba'; | |||
|
|
|||
| const fileChanges = await GitHub.readFileChanges([ | |||
| const fileChanges = await GitHub.readFileChanges(dir, [ | |||
There was a problem hiding this comment.
ChangedFiles are relative to dir so we really need both to correctly evaluate FileChanges
Context for SEEKers: https://seekchat.slack.com/archives/C03UM9GBGET/p1695369614026749