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

Fixes #8079 - Can update docker tags in repository #136

Merged
merged 1 commit into from Oct 31, 2014

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Oct 23, 2014

No description provided.

@jlsherrill
Copy link
Member

can you write a test?

# @param [Hash] tags for an image in the following format
# the [{:image_id => <image hash>, :tag =>"value"}]
# @return [RestClient::Response]
def update_docker_tags(repo_id, tags)
Copy link
Member

Choose a reason for hiding this comment

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

Missing documentation for the repo_id param

@parthaa
Copy link
Contributor Author

parthaa commented Oct 24, 2014

Well dont have a nice way to test this. Because we do not have a way to sync docker repos. I have created an issue that should address this http://projects.theforeman.org/issues/8102

@jlsherrill
Copy link
Member

I'm okay with skipping a test, but some need to be added before the docker work is complete

@ehelms
Copy link
Member

ehelms commented Oct 27, 2014

Still my comment to be addressed. And out of curiosity, why can't we sync a docker repo?

@parthaa
Copy link
Contributor Author

parthaa commented Oct 28, 2014

@ehelms updated as per your comment. Docker sync test for runcible is on way next. Discussing with bcourt on how to implement a dummy docker repo on local host similar to the way we do yum. If it turns out to be too complicated we'll resort to what puppet does point to @daviddavis puppet repos on fedorapeople.

@jlsherrill
Copy link
Member

@ehelms i don't think we can't, its juts that none of the previous docker PRs added any live tests, which was unfortunate.

@ehelms
Copy link
Member

ehelms commented Oct 28, 2014

:/ ACK

@parthaa
Copy link
Contributor Author

parthaa commented Oct 30, 2014

@ehelms @jlsherrill minor corrections. The CRUD repos has been tested live. Its just that sync has not been and hence the docker images have not been tested . I have an issue here to address that -> http://projects.theforeman.org/issues/8102

@jlsherrill
Copy link
Member

ACK

parthaa added a commit that referenced this pull request Oct 31, 2014
Fixes #8079 - Can update docker tags in repository
@parthaa parthaa merged commit 8a77c77 into Katello:master Oct 31, 2014
@parthaa parthaa deleted the tag branch October 31, 2014 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants