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

feat(#263) - Support for CHT Core 4.11 #264

Merged
merged 25 commits into from
Oct 25, 2024
Merged

Conversation

kennsippell
Copy link
Member

@kennsippell kennsippell commented Jul 29, 2024

#263

CHT Core 4.11.0 adds support for a new cht.v1.analytics object when viewing the contact summary of the user's facility. This change adds support for 4.11.0 and wires up this new object.

build.sh Outdated Show resolved Hide resolved
@kennsippell kennsippell changed the title feat(#263) - Support for CHT Core 4.8 feat(#263) - Support for CHT Core 4.11 Oct 18, 2024
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

Okay, sorry for the delay, this review turned out to be more involved then I expected... 😓

One additional thing is that we should be sure to update the compatibility matrix in the README.

build.sh Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
package.json Outdated
@@ -13,7 +13,7 @@
"docs": "npx jsdoc src/*.js src/**/*.js ./JSDOC.md -d docs",
"explore": "node ./project-explorer/build-assets.js --path=./test/collateral/project-without-source --formPath=./test/collateral && cd project-explorer && webpack && google-chrome ./project-explorer.html",
"eslint": "npx eslint .",
"build-ci": "npm run docs && npm run eslint && ./build.sh --force && npm test",
"build-ci": "npm run docs && npm run eslint && npm test",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"build-ci": "npm run docs && npm run eslint && npm test",
"build-ci": "npm run docs && npm run eslint && ./build.sh --force && npm test",

We should still be able to run the build script on CI right? At this point, even with --force, only 4.11.0 will be built since that is the only thing contained in cht_versions. If at all possible, I think it would be good to still run this just to make sure it works...

Copy link
Member Author

@kennsippell kennsippell Oct 24, 2024

Choose a reason for hiding this comment

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

I think it is important that the tests run against the files which are being committed into the dist folder. That is what will be distributed to end users.

I agree it is also important to test that the build script isn't broken. So I therefore will run build in a separate github action. Seem ok?

src/core-target-aggregates.js Outdated Show resolved Hide resolved
src/cht-script-api-factory.js Outdated Show resolved Hide resolved
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

👍 Looks great! LGTM!

@kennsippell kennsippell merged commit 74dac6d into master Oct 25, 2024
4 checks passed
@kennsippell kennsippell deleted the 4-8-feature-branch branch October 25, 2024 16:13
@medic-ci
Copy link

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants