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

new channels article #274

Closed
wants to merge 37 commits into from
Closed

new channels article #274

wants to merge 37 commits into from

Conversation

ringabout
Copy link
Member

No description provided.

@ghost
Copy link

ghost commented Mar 12, 2021

@ee7 can you proof-read please? :P

joinThreads(sender)
for i in 0 .. receiver.high:
createThread(receiver[i], recvHandler)
let start = now()
Copy link
Member

Choose a reason for hiding this comment

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

jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
jekyll/_posts/2021-03-12-new-Nim-channels.md Outdated Show resolved Hide resolved
@ringabout ringabout marked this pull request as ready for review March 31, 2021 10:07
@ee7
Copy link
Contributor

ee7 commented May 3, 2021

I see that @narimiran changed the date - does that mean this is going to be posted soon?

This article currently advertises "safe to use" quite a lot. But did we fix enough bugs in std/channels? Maybe we should use a more conservative wording if we're not fully confident yet. See e.g. nim-lang/Nim#17392

@dom96
Copy link
Contributor

dom96 commented May 3, 2021

The new channels library is efficient and safe to use

Why even say this? It implies that it has a reputation as not being safe to use. Just omit this wording completely, the assumption is that libraries are safe to use when they are being advertised as "ready for use".

Copy link

@ZoomRmc ZoomRmc left a comment

Choose a reason for hiding this comment

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

  1. All the examples in the article use globals for channels, what's worse, the only example in the module docs does the same. I understand it's the simplest way, but shouldn't we say a couple of words on how to properly use them without global variables?

  2. Is the article planned to be released when channels gets out of devel? Will the old one get deprecated simultaneously? In any case, there should be an explicit mention on how to adapt the code which uses system.Channel to the new one, even if it's mere Just remove X,Y and Z and you're done!

@ringabout ringabout closed this Jul 10, 2021
@ringabout ringabout reopened this Jul 12, 2021
@ringabout ringabout marked this pull request as draft July 12, 2021 10:45
@ringabout
Copy link
Member Author

Since the channel module may be rewritten/redesigned, this article will become a bit irrelevant. So I mark it as a draft.

@GordonBGood
Copy link
Contributor

Since the channel module may be rewritten/redesigned, this article will become a bit irrelevant. So I mark it as a draft.

Yes, I am in process of re-writing the module as the old one has a lot of problems including one or two data races. It's almost finished. When it has been accepted, I'll revisit this to see if this document reflects what the actual module has become.

@ringabout ringabout closed this by deleting the head repository Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants