chore: better logging in automap plugin#718
Conversation
WalkthroughThis pull request introduces enhancements to the logging capabilities of the Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
plugins/automap/src/automap.service.ts (4)
93-101: Enhance error message with source sheet detailsWhile the error logging is well-placed, the message could be more informative by including details about the source sheet to aid in debugging.
if (this.isNil(destinationSheetId)) { if (this.options.debug) { logError( '@flatfile/plugin-automap', - 'Unable to determine destination sheet' + `Unable to determine destination sheet for target "${target}" (source sheet: ${source})` ) } return }
168-173: Consider using logWarn instead of logErrorThe absence of workbooks might be an expected scenario in some cases, making it more appropriate to use
logWarninstead oflogError. This would better reflect that this is a validation state rather than a system error.if (targets.length === 0) { if (this.options.debug) { - logError( + logWarn( '@flatfile/plugin-automap', 'No workbooks found, skipping automap' ) } return undefined
350-355: Add context to the warning messageThe warning message could be more helpful by explaining why the mapping couldn't be determined (multiple sheets present and no selection function).
if (this.options.debug) { logWarn( '@flatfile/plugin-automap', - 'Unable to determine mapping between source and destination sheets, skipping automap' + `Unable to determine sheet mapping: found ${sheets.length} sheets but no sheet selection function provided, skipping automap` ) }
Line range hint
1-3: Extract module name to a constantTo maintain consistency and avoid typos, consider extracting the module name used in logging statements to a constant.
import { Flatfile, FlatfileClient } from '@flatfile/api' import type { FlatfileEvent, FlatfileListener } from '@flatfile/listener' import { logError, logInfo, logWarn } from '@flatfile/util-common' + +const MODULE_NAME = '@flatfile/plugin-automap'Then use it in all logging calls:
-logError('@flatfile/plugin-automap', 'No Workbook Id found') +logError(MODULE_NAME, 'No Workbook Id found')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.changeset/proud-snails-jam.md(1 hunks)plugins/automap/src/automap.service.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/proud-snails-jam.md
Please explain how to summarize this PR for the Changelog:
This PR adds more logging to the automap plugin
Closes https://github.com/FlatFilers/support-triage/issues/1704
Tell code reviewer how and what to test: