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.

Sign up for GitHub

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

Jump to bottom

feat: ngc integrator charm for NGC with Notebooks #2

Merged

NohaIhab merged 14 commits into main from kf-5162-add-ngc-integrator-charm

Feb 2, 2024

Contributor

NohaIhab commented Jan 19, 2024

Adds the new charm ngc-integrator


          feat: initial commit

2f5f471

github-actions bot added the Libraries: Out of sync label

NohaIhab changed the title ~~feat: ngc integrator charm for Jupyter~~ feat: ngc integrator charm for NGC with Notebooks

NohaIhab added 9 commits

January 19, 2024 14:25


          fix: lint

a6f0cb0


          feat: upgrade instructions

e8f5d81


          fix: formatting dependencies

a5d3052


          fix: ununsed imports

a76fe43


          feat: add unit tests and point integration tests to pr channel

d71308c


          fix: integration tests

336d1ab


          fix: poddefault details according to spec

77ac9b7


          feat: assert unit test error msg

bf9b3c4


          fix: format

917ccd1

DnPlas reviewed

View reviewed changes

Contributor

DnPlas left a comment

Thanks @NohaIhab, I did a first pass on the PR and left some comments.

.github/workflows/integrate.yaml Outdated Show resolved Hide resolved

.github/workflows/integrate.yaml Show resolved Hide resolved

.github/workflows/integrate.yaml Outdated Show resolved Hide resolved

CONTRIBUTING.md Outdated Show resolved Hide resolved

LICENSE Outdated Show resolved Hide resolved

src/charm.py Outdated Show resolved Hide resolved

src/components/manifests_relation_component.py Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved


          fix: address comments

e4403cf

github-actions bot added Libraries: OK and removed Libraries: Out of sync labels

DnPlas reviewed

View reviewed changes

Contributor

DnPlas left a comment

Did a second pass, thanks @NohaIhab !

tests/integration/test_charm.py Outdated Show resolved Hide resolved

src/components/manifests_relation_component.py Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved

metadata.yaml Outdated Show resolved Hide resolved

src/components/manifests_relation_component.py Outdated Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved

tests/integration/crds/poddefaults.yaml Outdated Show resolved Hide resolved

tests/integration/test_charm.py Outdated Show resolved Hide resolved

tests/unit/test_charm.py Outdated Show resolved Hide resolved


          address second review

fa63518

github-actions bot added Libraries: Out of sync and removed Libraries: OK labels

DnPlas reviewed

View reviewed changes

Contributor

DnPlas left a comment

Just a few comments, other than that LGTM. Thanks @NohaIhab !

tests/unit/test_charm.py Outdated Show resolved Hide resolved

src/components/manifests_relation_component.py Show resolved Hide resolved

NohaIhab added 2 commits

January 31, 2024 12:03


          feat: assert error status in unit tests

73e49eb


          fix: test docstring

27db582

DnPlas approved these changes

View reviewed changes

Contributor

DnPlas left a comment

Thanks @NohaIhab, LGTM. Please note there are two unresolved conversations that require your action. I did not resolve them to not forget about them. Feel free to close them once you've completed each of the tasks.

NohaIhab merged commit 01ac297 into main

6 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Libraries: Out of sync