Repository navigation
Varo enhancements & Gmail documentation update - #1348
Conversation
…multiple Varo packages. Enhanced Gmail adaptor documentation with better examples.
josephjclark
left a comment
josephjclark
left a comment
There was a problem hiding this comment.
A couple of admin points and a question over job code vs adaptor code
| if (tambAlrm != null) { | ||
| mergedRecord['zTambAlrm'] = tambAlrm; | ||
| mergedRecord['ALRM'] = tambAlrm; | ||
| // mergedRecord['ALRM'] = tambAlrm; // tambAlrm is unused in this workflow. |
There was a problem hiding this comment.
Alarm bells are ringing a bit here - adaptors should not be coupled to workflows. They should be generic. This varo adaptor has always been a bit unusual in that regard, but I remain concerned.
If different workflows need to process data differently, that needs to be exposed as options in the operation and specified in job code.
At the very least, this line and comment need to be removed because they give the wrong impression of the adaptor.
More broadly, I think you need to consider moving some of these utils out of adaptor code and into the workflow. I'm very interested to know what's blocking you from that - why are you reaching for the adaptor to process this business logic, rather than doing it in the job code where a) it's visible to project admins/auditors, and b) it can be tailored to a specific workflow?
There was a problem hiding this comment.
You're right, the wording here is misleading considering 'workflow' being a loaded term in this context. I've clarified it to make it explicit that omitting this property is intentional, not accidental.
Stepping back, the intent of the Varo adaptor is to encapsulate an interpretation of the business logic so that multiple sovereign entities can implement consistent solutions in their own workflows across different platforms. Centralizing the logic in the adaptor allows it to be maintained once and distributed via versioning, rather than re-implemented in each workflow. To me, this feels like an appropriate use case for an adaptor.
There was a problem hiding this comment.
So long as these utils work (at least conceptually!) across different workflows then I have no problem :)
Appreciate the comment update
Either way, in practice this is your workflow right now and I'm quite happy to give you the benefit of the doubt and prioritise solving your business needs.
fixed typo in readme moved methods from Adaptor.js fixed misleading comment enhanced unit tests for main convertEms function added fixture for unit tests.
2 participants
Summary
Varo adaptor function enhancement and Gmail adaptor documentation update.
Fixes #
Details
Enhanced Varo adaptor to have better RTCW resolution when processing multiple Varo packages.
Enhanced Varo adaptor documentation to explain RTCW enhancement.
Enhanced Gmail adaptor documentation with better examples.
Add technical details of what you've changed (and why).
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to
know!):
You can read more details in our
Responsible AI Policy
Review Checklist
Before merging, the reviewer should check the following items:
production? Is it safe to release?
dev only changes don't need a changeset.