-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add Crystal::HOST_TRIPLE
and TARGET_TRIPLE
#13823
Add Crystal::HOST_TRIPLE
and TARGET_TRIPLE
#13823
Conversation
spec/compiler/config_spec.cr
Outdated
@@ -6,6 +6,7 @@ describe Crystal::Config do | |||
{% begin %} | |||
# TODO: CRYSTAL_SPEC_COMPILER_BIN must be quoted (#11456) | |||
{% compiler = (env("CRYSTAL_SPEC_COMPILER_BIN") || "bin/crystal").id %} | |||
# TODO: check against Crystal::HOST_TRIPLE instead |
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 would like to add some context as to when this TODO should be applied. It's effectively after we drop forward compatibility with Crystal < 1.10 (assuming HOST_TARGET
gets released in 1.10).
Not sure how to best express that though...
Maybe we could add a macro to already use HOST_TRIPLE
when available? ({% if Crystal.has_constant?(:HOST_TRIPLE) %}
)
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.
We could also put the Crystal::
constant tests inside the primitive specs since they require a fresh compiler. Not sure if that'll look odd to some people
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.
Yeah, such specs could be a supplemenatry addition.
This spec is just using the constant for calculating the expected value, it's not testing the constant itself.
Resolves #13156.