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

added defaultDeploymentRepo on RepositoryVirtual #461

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

PatriceG
Copy link

I added support to set the default deployment repo name, that we need.
It's fixing issue #460

Copy link
Member

@beliaev-maksim beliaev-maksim left a comment

Choose a reason for hiding this comment

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

also need tests for this

dohq_artifactory/admin.py Outdated Show resolved Hide resolved
@PatriceG
Copy link
Author

PatriceG commented Dec 12, 2024

sorry indeed it should be
self.default_deployment_repo_name = default_deployment_repo_name

I pushed a fix.

I'm not sure I understand how to setup a test environment for your lib.

@beliaev-maksim
Copy link
Member

please provide a unittest with mocks, we use responses

@PatriceG
Copy link
Author

PatriceG commented Dec 13, 2024

I'm not familiar with mocking in Python.
I'm not sure I understand how to setup a test environment for your lib.
I just wanted to contribute this fix in the lib (I do have a workaround by sublcassing the RepositoryVirutal class but it's not ideal).

@beliaev-maksim
Copy link
Member

these are the classic unit tests:
https://github.com/devopshq/artifactory/blob/master/docs/CONTRIBUTE.md#unit

then you can see the unit folder and add new tests similar to other

@PatriceG
Copy link
Author

There is not a single existing unit test for RepositoryLocal and Virtual.
I'm sorry I have no time to try to understand how to setup a test env.
Feel free to reject my PR if you like. I can do with my local modifications. I just wanted to simply contribute.

@allburov allburov merged commit 3c21d18 into devopshq:master Dec 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants