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

Expose is_set #99

Merged
merged 2 commits into from
Feb 25, 2020
Merged

Expose is_set #99

merged 2 commits into from
Feb 25, 2020

Conversation

ailisp
Copy link
Contributor

@ailisp ailisp commented Feb 22, 2020

In a situation I need check if there's a currently running actix system, but not panic when there isn't. This pub crate function just works and seems not dangerous to expose it.

Copy link
Member

@Jonathas-Conceicao Jonathas-Conceicao left a comment

Choose a reason for hiding this comment

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

Makes sense to me to have it publicly available. Left a suggestion for the doc.

Also, could you update the actix-rt/CHANGES.md?

actix-rt/src/system.rs Outdated Show resolved Hide resolved
Copy link
Member

@Jonathas-Conceicao Jonathas-Conceicao left a comment

Choose a reason for hiding this comment

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

LGTM!

@Jonathas-Conceicao Jonathas-Conceicao merged commit 602db17 into actix:master Feb 25, 2020
@JohnTitor
Copy link
Member

@Jonathas-Conceicao On some repositories (actix, actix-net, actix-web), merge should be done with care. And I didn't review this yet...

@Jonathas-Conceicao
Copy link
Member

@JohnTitor not sure what I did wrong here. But I will start to wait on your review before merging in the future.

@JohnTitor
Copy link
Member

@Jonathas-Conceicao Don't have to wait though, but you could ping me when API is changed, it'd be great! Otherwise, I may miss the change :)

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.

4 participants