-
Notifications
You must be signed in to change notification settings - Fork 937
test-in-docker: fix with newer ZSH versions #884
test-in-docker: fix with newer ZSH versions #884
Conversation
The way I was filtering out entries in the frameworks array stopped working in newer versions of ZSH; it would convert the array into a string (you could see it with `typeset -p frameworks`) So I rewrote it. I don't see anything in the release notes for ZSH that would explain this and I didn't find any option that would restore this behavior. Related: #882
Marking variables as readonly is helpful for debugging and preventing problems.
@dritter I wouldn't mind a review on this, if you have time. |
test-in-docker
Outdated
frameworks=${(@)frameworks:#base-*} | ||
for i in {$#frameworks..1}; do | ||
# Remove all base entries | ||
[[ "${frameworks[$i]}" = base-* ]] && frameworks[$i]=() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a stylistic note: I would go here with double equals. That is more readable IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 3c27414
test-in-docker
Outdated
@@ -6,6 +6,7 @@ set -eu | |||
default_version='4.3.11' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should set the minimum version to something like 5.3 in regard of #877
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started to do that in a94df3d but then remembered why I have it the way I do.
The default should be the oldest version of zsh we support, not newest.
Either way, it is easier to change now and it has the same lookup properties the command line does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well. The oldest supported version of ZSH is 5.3 in next
(at least in the battery segment). Otherwise it is 5.1.
Btw. Did you see my questions in the chat? |
@@ -1,4 +1,4 @@ | |||
FROM ubuntu:17.04 | |||
FROM ubuntu:17.10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. 17.04 (and 17.10) are not LTS versions; they will go away.
Merged into master as part of #944, and will be part of the v0.6.6 release! |
The way I was filtering out entries in the frameworks array stopped working in newer versions of ZSH; it would convert the array into a string (you could see it with
typeset -p frameworks
)So I rewrote it.
I don't see anything in the release notes for ZSH that would explain this and I didn't find any option that would restore this behavior.
I also added a bunch of debugging and warning stuff to catch problems in the future.
And there is a
--dry-run
flag now.Related: #882 (cc: @dritter)