Skip to content

Add Support for --label flags#58

Merged
nexdrew merged 6 commits intonexdrew:masterfrom
hertzg:master
May 20, 2020
Merged

Add Support for --label flags#58
nexdrew merged 6 commits intonexdrew:masterfrom
hertzg:master

Conversation

@hertzg
Copy link
Contributor

@hertzg hertzg commented Apr 18, 2020

Adds support for --label flags and introduces a new utility method appendObjectEntries to transform

            "Labels": {
                // ... JSON Object with string key and values
                "traefik.enable": "true",
                "traefik.http.services.some-name.loadbalancer.server.port": "1337"
            }

into

--label 'traefik.enable'='true' --label 'traefik.http.services.some-name.loadbalancer.server.port'='1337'

* Add support for --label flag
Copy link
Owner

@nexdrew nexdrew left a comment

Choose a reason for hiding this comment

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

@hertzg Hello 👋

Thanks for submitting this, I apologize for the very long delay in getting your PR reviewed!

I have a couple questions/suggestions, see separate comments below.

Also, we will need to update the expected docker run command in test-cli.js here (i.e. need to add label arguments between these two lines):

rekcod/test/test-cli.js

Lines 36 to 37 in 8dae628

'--expose 4700/tcp --expose 4702/tcp ' +
'-e \'DB_USER=postgres\' ' +

If you're still interested in getting this PR merged, let me know what you think. Otherwise, I can recreate your changes on another branch and try to get this merged/released, so everyone using rekcod will get the benefit of your work.

Copy link
Owner

@nexdrew nexdrew left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

I'm gonna merge this as is. If I want any further changes, I'll add them in a follow-up PR.

@nexdrew nexdrew merged commit f2fc146 into nexdrew:master May 20, 2020
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