Skip to content

Conversation

mattmikolay
Copy link
Contributor

@mattmikolay mattmikolay commented Jun 23, 2025

Description

This PR removes the withReadonlyField option from the various functions used to read resource directories. It appears we never set withReadonlyField to true anywhere within the codebase.

Tasks

KNO-8961

Copy link
Contributor Author

mattmikolay commented Jun 23, 2025

@mattmikolay mattmikolay changed the title Remove withReadonlyField option chore(KNO-8961): remove withReadonlyField option Jun 23, 2025
Copy link

linear bot commented Jun 23, 2025

@mattmikolay mattmikolay marked this pull request as ready for review June 23, 2025 20:42
@mattmikolay mattmikolay requested review from connorlindsey and thomaswhyyou and removed request for connorlindsey June 23, 2025 20:42
Copy link
Contributor

@connorlindsey connorlindsey left a comment

Choose a reason for hiding this comment

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

lgtm although I'm not 100% sure what the intended purpose of read only fields is in the CLI

opts: ReadLayoutDirOpts = {},
): Promise<ParseJsonResult | JoinExtractedFilesResult> => {
const { abspath } = layoutDirCtx;
const { withExtractedFiles = false, withReadonlyField = false } = opts;
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears we never set withReadonlyField to true anywhere within the codebase.

It's been awhile so I'm trying to remember (😅) but I think I provided these options for completeness early on, to mirror the annotation directive which controls how a resource payload gets transformed and written into a json file.

And conceptually these "read" functions do the inverse, so I probably thought having options to control how the read behavior works with respect to extractable and readonly fields made sense. But it turned out the commands we have (so far at least) that reads from the file system haven't necessarily needed to care about the read only fields .

@mattmikolay mattmikolay merged commit 4a94b63 into main Jun 30, 2025
8 checks passed
@mattmikolay mattmikolay deleted the mattmik-kno-8961-cli-remove-withreadonlyfield-option-from-directory-read branch June 30, 2025 13:39
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