Simplify logReadFiles. NFC
#26186
Simplify logReadFiles. NFC
#26186
Conversation
| FS.chmod(node, mode & 0o777); | ||
| } | ||
| #if expectToReceiveOnModule('logReadFiles') | ||
| #if FS_DEBUG |
There was a problem hiding this comment.
Is this PR changing the requirement of FS_DEBUG, or just documenting it? From the description I thought the latter, but the code seems to actually add it..?
There was a problem hiding this comment.
The log message was already wrapped in FS_DEBUG yes
There was a problem hiding this comment.
I think maybe there was some confusion about how this feature worked. See #7418, where the log message was changed.
The log message is the feature really. Since it was already wrapped in FS_DEBUG I think its best to document that. I seems reasonable to run in FS_DEBUG mode if want to get a list of opened files.
There was a problem hiding this comment.
Aside from the log there is old line 1166 though, FS.readFiles[path] = 1? I think users might have just used that, without the logging?
There was a problem hiding this comment.
Perhaps? Seems like we should probably keeps this as just one feature though. Bifurcating it further seems like complexity we don't need. Putting the whole feature behind FS_DEBUG seems like the most sensible thing to do.
Also, document that `logReadFiles` requires `FS_DEBUG` to be set.
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
I updated the PR description.
2 participants
logReadFilesrequiresFS_DEBUGto be set.FS.openedFiles) behingFS_DEBUGinstead of only the former.The actual string that was logged was changed in #7418, but I think it was in error.