[ADD] OpenSPP adaptor #409
Conversation
There was a problem hiding this comment.
hi @nhatnm0612 , thanks for this! a few quick things before this gets to code review:
- check your package.json... if nothing else, the name should be language-openspp (not "template")
- check your changelog! (this should be the first entry.)
- check the configuration schema... is this the right way to set up a credential for use with OpenSPP?
otherwise, at first glance, this looks really cool!
hi @taylordowns2000 , thanks for your suggestions, I have fixed these |
There was a problem hiding this comment.
hi @nhatnm0612 , this is looking really good. a few small changes and then one conversation to be had with @josephjclark or @mtuchi about how you're doing authentication and then passing around a connection.
i'd recommend that you either go with the approach shown in the salesforce adaptor or wait for Joe's feedback on your style here.
thanks again. this is looking really good! is it all working as expected with the new CLI?
There was a problem hiding this comment.
replace with openspp rectangular logo 512x190
There was a problem hiding this comment.
@taylordowns2000 hi, I did change the logos
There was a problem hiding this comment.
This new image is 567x120. @taylordowns2000 how precise do we need to be about the logo size?
There was a problem hiding this comment.
this is the best I could find in the weekend, I will ask someone during the next week
There was a problem hiding this comment.
I'll edit something together for you on Monday
There was a problem hiding this comment.
This is looking much better @nhatnm0612! Thank you for your efforts and your patience.
I've spotted a few more things that look important so please address the outstanding comments when you can.
I would love to be able to do some sort of testing of this. I don't know OpenSPP and there are no unit tests (obviously unit tests are hard because they rely on the OpenSPP backend, so don't worry about those).
Would it be possible to write a sample job which runs against the OpenSPP sandbox and exercises a few of these functions? You can call it job.example.js and include it under the test directory.
There was a problem hiding this comment.
This new image is 567x120. @taylordowns2000 how precise do we need to be about the logo size?
…ate in logs, fix README file
There was a problem hiding this comment.
Thank you again, we're very very nearly there!
Two small tweaks to the readme. I'll edit you that image - I'm not sure if I can add it directly to your fork but I'll try it as soon as I get a chance.
The biggest one is we still need some kind of sample code which runs against the OpenSPP sandbox. I'd really like to run some kind of test on this before we merge it. Just one or two operation calls would suffice.
about the image, I am asking for the correct size, please wait. |
|
hi @taylordowns2000 @josephjclark , I updated the image & also did some test cases, please take a look, thank you all for your patient with a javascript newbie like me 😄 |
|
@nhatnm0612 thank you for the image! And your Javascript is doing great, it's been a pleasure to work with you on this |
nhatnm0612
force-pushed
the
add-openspp-adaptor
branch
from
f449738
to
19fe85c
Compare
thanks, a lot, for all the efforts you paid to me |
|
Hi @nhatnm0612 - this adaptor is now released and available to the world! https://www.npmjs.com/package/@openfn/language-openspp |
Summary
Add new adaptor for OpenSPP
Details
Create new adaptor following documents.
Review Checklist
Before merging, the reviewer should check the following items:
migration guide followed?
dev only changes don't need a changeset.