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

Support for HiPE compiler #65

Closed
wants to merge 1 commit into from

Conversation

astawiarski
Copy link
Contributor

Added support for enabling HiPE compiler through RABBITMQ_HIPE_COMPILE environment variable

@astawiarski
Copy link
Contributor Author

I've created a corresponding pull request in documentation: docker-library/docs#476

cat >> /etc/rabbitmq/rabbitmq.config <<-EOC
{$conf, $val},
EOC
fi
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the delay! I'm +1 on the HiPE change in general (making it available conditionally), but I think this block could use some improvement, and here's what I'd propose instead:

for conf in "${configs[@]}"; do
    var="RABBITMQ_${conf^^}"
    val="${!var}"
    rawVal=
    case "$conf" in
        # SSL-related options are configured above, so should be ignored here
        ssl_*) continue ;;

        # convert shell booleans into Erlang booleans
        hipe_compile)
            [ "$val" ] && rawVal='true' || rawVal='false'
            ;;

        # otherwise, assume string-based (and skip or add appropriate decorations)
        *)
            [ "$val" ] || continue
            rawVal='<<"'"$val"'">>'
            ;;
    esac
    cat >> /etc/rabbitmq/rabbitmq.config <<-EOC
        {$conf, $rawVal},
    EOC
done

(I'm happy to take over the PR from here as well, if you'd rather, taking your commits and making a new PR that implements this case. 👍)

@astawiarski
Copy link
Contributor Author

Thanks for the comment and suggestion, it's a great lesson! Please feel free to take over

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