Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commcare: update documentation #814

Merged
merged 6 commits into from
Nov 4, 2024
Merged

Conversation

hunterachieng
Copy link
Contributor

@hunterachieng hunterachieng commented Nov 4, 2024

Summary

Update commcare documentation with more examples and better formatting

Fixes #775

Details

Add more examples and update the commcare documentation

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

  • Does the PR do what it claims to do?
  • If this is a new adaptor, added the adaptor on marketing website ?
  • Are there any unit tests?
  • Is there a changeset associated with this PR? Should there be? Note that
    dev only changes don't need a changeset.
  • Have you ticked a box under AI Usage?

Signed-off-by: Hunter Achieng <[email protected]>
@hunterachieng hunterachieng force-pushed the feature/775-update-commcare-docs branch from 55f99ce to df819db Compare November 4, 2024 14:42
Copy link
Collaborator

@josephjclark josephjclark left a comment

Choose a reason for hiding this comment

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

Just a bit of pasted code that needs removing please

packages/commcare/src/Adaptor.js Outdated Show resolved Hide resolved
Signed-off-by: Hunter Achieng <[email protected]>
Copy link
Collaborator

@josephjclark josephjclark left a comment

Choose a reason for hiding this comment

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

Looks good

Aleksa said something about configuration in the issue - is that anything that needs addressing here?

@hunterachieng
Copy link
Contributor Author

Looks good

Aleksa said something about configuration in the issue - is that anything that needs addressing here?

Not really, the configuration docs is accurate and up to date

@josephjclark
Copy link
Collaborator

josephjclark commented Nov 4, 2024

@hunterachieng what about the Sample Configuration? Should that have a user/pass or apikey? Just as a starting point for users. Maybe an app id too?

@josephjclark This has been fixed

@josephjclark
Copy link
Collaborator

@hunterachieng Oh I see, the sample configuration comes from required keys. hmm.

We can't make username and password required, because they're not required if you pass an API key. So we'll have to live without those in the example.

I presume appId is always required though?

What about hostURL? Is that really required or can we (do we?) safely default that to commcarehq?

@hunterachieng
Copy link
Contributor Author

@hunterachieng Oh I see, the sample configuration comes from required keys. hmm.

We can't make username and password required, because they're not required if you pass an API key. So we'll have to live without those in the example.

I presume appId is always required though?

What about hostURL? Is that really required or can we (do we?) safely default that to commcarehq?

@josephjclark hostURL is a required field

@josephjclark
Copy link
Collaborator

Ok @hunterachieng , thank you! I'll get this merged in a minute

@josephjclark josephjclark merged commit 167ad45 into main Nov 4, 2024
2 checks passed
@josephjclark josephjclark deleted the feature/775-update-commcare-docs branch November 4, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

commcare: update docs and add examples
2 participants