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

fix RFCs/411: add std/cputicks #18743

Closed
wants to merge 6 commits into from

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Aug 24, 2021

fix nim-lang/RFCs#411
(split off from #18149 to just focus on adding std/cputicks)

lib/std/cputicks.nim Outdated Show resolved Hide resolved
lib/std/cputicks.nim Outdated Show resolved Hide resolved
lib/std/cputicks.nim Show resolved Hide resolved
lib/std/cputicks.nim Outdated Show resolved Hide resolved
lib/std/monotimes.nim Outdated Show resolved Hide resolved
@timotheecour
Copy link
Member Author

PTAL

@Varriount
Copy link
Contributor

Varriount commented Aug 25, 2021

Please only open these PRs once actual discussion has occurred on their RFCs - otherwise it creates unnecessary noise for reviewers and risks fracturing discussion. A link to the implementation can be given in the RFC.

I would also recommend using "implements" or "resolves" rather than "fixes" for PRs like this. "fixes" implies that a PR solves an objective problem, such as a logic bug, rather than a subjective problem.

@timotheecour
Copy link
Member Author

timotheecour commented Aug 26, 2021

Please only open these PRs once actual discussion has occurred on their RFCs - otherwise it creates unnecessary noise for reviewers and risks fracturing discussion.

I disagree. An RFC becomes much more concrete with a link to a PR that implements it, which users can try and break. This is how you refine ideas and identify problems, otherwise it just remains a vague idea. You can comment on the RFC for the general design/idea, and on the PR for the particulars of the implementation. A link

A link to the implementation can be given in the RFC.

which is what this PR is, allowing inline comments etc

I would also recommend using "implements" or "resolves" rather than "fixes" for PRs like this. "fixes" implies that a PR solves an objective problem, such as a logic bug, rather than a subjective problem.

potato, potato. implements doesn't work with github issue cross-linking.

@arnetheduck
Copy link
Contributor

"draft PR":s exists for this reason

@arnetheduck
Copy link
Contributor

or they can sit in a fork repo, and have equally good integration with github while still reducing noise

@Araq
Copy link
Member

Araq commented Mar 24, 2022

This should start its life as a Nimble package. Also: Supporting this at compile-time from the get go is not nice, not everybody enjoys the possibility of a build depending on CPU ticks.

@Araq Araq closed this Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review (please take another look): ready for next review round
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std/cputicks: high resolution low overhead strictly monotonic cpu counter
5 participants