Skip to content

Conversation

@JCGoran
Copy link
Contributor

@JCGoran JCGoran commented Mar 11, 2021

Fixes #41.
Instead of Git tags, it now parses Dockerhub tags instead.
Additionally, now it always uses 'latest' as the default tag.

Instead of Git tags, it now parses Dockerhub tags instead.
Additionally, now it always uses 'latest' as the default tag.
Copy link
Owner

@schuellerf schuellerf left a comment

Choose a reason for hiding this comment

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

The changes do make sense to me and I'll test and merge if I find more time than today...

xfce-test Outdated
LOG=${LOG:-/dev/stdout}
PARALLEL_BUILDS=${PARALLEL_BUILDS:-0}
TRAVIS=${TRAVIS:-FALSE}
TAG='latest'
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer if it can be overridden if needed

Suggested change
TAG='latest'
TAG=${TAG:-latest}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right, fixing it now.

xfce-test Outdated
return 0
fi
done
# the specified tag doesn't exist on Dockerhub
Copy link
Owner

Choose a reason for hiding this comment

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

You could move the error message and printing of all tags which are available here.
(or you wanted to make the call more encapsulated?)
Either way I think printing the list of available tags, if not found, would improve the usability alot

@JCGoran
Copy link
Contributor Author

JCGoran commented Mar 15, 2021

Okay, your first suggestion was simple to fix, but the second was a bit trickier, and I will admit that my solution is somewhat hacky.
That said, the following used to break the script:

# invalid tag (unless it's available locally)
$ export TAG='asdf' && ./xfce-test start
# invalid Dockerhub tag
$ ./xfce-test pull asdf

Now it doesn't break anymore, and the output with an invalid tag looks something like this:

$ export TAG='asdf' && ./xfce-test start
The tag 'asdf' was not found on Dockerhub
---
The following tags are available on Dockerhub:
latest
audio_test
...
ubuntu_20.04-xfce-4.16pre2
ubuntu_xfce-4.16pre1
xubuntu_19.04
Attempting to find the tag 'asdf' locally...
The tag 'asdf' was not found locally
---
The following tags are available locally:
latest
ubuntu_20.04-xfce-4.16pre2
Unable to find the tag 'asdf' either on Dockerhub or locally, exiting...

while the pull version outputs just the Dockerhub tags and exits gracefully.
This is a bit lengthy, but could be fixed separately by adding a verbosity flag or something to the script itself.

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.

Unable to start or pull (broken tag?)

2 participants