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: adding lsf-platform to compute envs #57

Merged
merged 3 commits into from
Jan 19, 2024

Conversation

timini
Copy link
Contributor

@timini timini commented Jan 12, 2024

This PR adds support for IBM LSF compute envs

@alexhermida
Copy link
Member

Hey @timini ! Thanks for the contribution 🙂 would be great if you can add at least one test here for the compute environment.
Thank you again!

@timini
Copy link
Contributor Author

timini commented Jan 15, 2024

Sorry, couldn't find the test suite, will add.

@alexhermida
Copy link
Member

Sorry, couldn't find the test suite, will add.

No problem! The repo it's not great documented for external contributors, something to work on😄

@alexhermida
Copy link
Member

alexhermida commented Jan 16, 2024

Hi @timini you need to run locally go generate ./... and commit the changes with the new docs.
As I said the README.md needs to be updated for contributors 😄

Thanks!

@timini
Copy link
Contributor Author

timini commented Jan 19, 2024

@alexhermida can you kick off the tests again please?

@timini
Copy link
Contributor Author

timini commented Jan 19, 2024

hi @alexhermida I'm interested in adding secrets support as a separate MR (sorry couldn't find a way to open an issue for discussion). How would i go about testing it locally, does it have to be done against a live instance of tower?

@alexhermida
Copy link
Member

alexhermida commented Jan 19, 2024

in adding secrets support as a separate MR (sorry could

What do you mean adding secrets support? As it's right now already requires NFTOWER_API_KEY. I think the only test we currently have around that is to test if it's required.

If it's ok. I'll merge this and feel free to open a new MR with the code, we can discuss there if it's appropiate, probably is easier with the code 😄

Thanks!

@alexhermida alexhermida enabled auto-merge (squash) January 19, 2024 12:27
@alexhermida alexhermida merged commit 7994981 into healx:main Jan 19, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants