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

bashbrew: account for namespaces when sorting repos #6054

Merged
merged 3 commits into from
Jun 15, 2019
Merged

bashbrew: account for namespaces when sorting repos #6054

merged 3 commits into from
Jun 15, 2019

Conversation

7AC
Copy link
Contributor

@7AC 7AC commented Jun 7, 2019

When sorting repos sort.go looks at the FROM lines in the Dockerfiles. If --namespace is set it needs to be taken into account as the line will be in the form FROM <namespace>/<image>[:<tag>]. This also adds --namespace to bashbrew list as it also sorts.

7AC added a commit to aristanetworks/official-images that referenced this pull request Jun 7, 2019
@tianon
Copy link
Member

tianon commented Jun 11, 2019

Heh, I debated doing this exact thing back and forth a lot when I originally wrote bashbrew list (hence why there's a namespace := "" in there as a placeholder of sorts).

The amount of places we pass this around has me wondering if maybe it's time to instead move --namespace to be a global option on par with --arch so we don't have to pass it around so much (and because it needs to affect basically everything anyhow). The only real exception is bashbrew tag, which needs to take the non-namespaced images and tag them into a different namespace, so it would likely just adjust to have a --target-namespace parameter or something similar.

I need to noodle on this a little more. 👍 ❤️

@tianon
Copy link
Member

tianon commented Jun 13, 2019

I gave it more thought and took a stab at porting your implementation (especially now that #5929 is finally resolved). 👍

@7AC
Copy link
Contributor Author

7AC commented Jun 14, 2019

Thanks @tianon ! Anything else I need to do to merge?

@tianon
Copy link
Member

tianon commented Jun 14, 2019 via email

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.

3 participants