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

feature: versioned interrupt signal #67

Merged
merged 2 commits into from
May 11, 2024
Merged

Conversation

grkek
Copy link
Member

@grkek grkek commented May 11, 2024

No description provided.

@grkek
Copy link
Member Author

grkek commented May 11, 2024

@yanecc What do you think of this feature, I think it is enough to have a versioned interrupt handler?

@grkek grkek merged commit 49c2a9e into core May 11, 2024
1 check passed
@grkek grkek deleted the feature/versioned_interrupt_signal branch May 11, 2024 13:18
@yanecc
Copy link
Contributor

yanecc commented May 11, 2024

Process.on_interrupt is available since crystal 1.8.0.

@yanecc
Copy link
Contributor

yanecc commented May 11, 2024

Crystal::VERSION.split(".").map(&.to_i) will cause unhandled exception when building with crystal versions under development such as 1.13.0-dev and you may like:

{% if compare_versions(Crystal::VERSION, "1.12.0") >= 0 %}
  ...
{% elsif compare_versions(Crystal::VERSION, "1.8.0") < 0 %}
  ...
{% else %}
  ...
{% end %}

@grkek
Copy link
Member Author

grkek commented May 12, 2024

Crystal::VERSION.split(".").map(&.to_i) will cause unhandled exception when building with crystal versions under development such as 1.13.0-dev and you may like:

{% if compare_versions(Crystal::VERSION, "1.12.0") >= 0 %}
  ...
{% elsif compare_versions(Crystal::VERSION, "1.8.0") < 0 %}
  ...
{% else %}
  ...
{% end %}

You are right, I will add a check to remove such cases from the starting string.

@yanecc
Copy link
Contributor

yanecc commented May 12, 2024

Process.on_interrupt is available since crystal 1.8.0.

@grkek

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.

None yet

2 participants