Skip to content

Initial code export#1

Merged
eholk merged 2 commits intomicrosoft:mainfrom
eholk:initial-code-export
Jul 21, 2022
Merged

Initial code export#1
eholk merged 2 commits intomicrosoft:mainfrom
eholk:initial-code-export

Conversation

@eholk
Copy link
Copy Markdown
Contributor

@eholk eholk commented Jul 21, 2022

No description provided.

Copy link
Copy Markdown
Contributor

@daprilik daprilik left a comment

Choose a reason for hiding this comment

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

excited to see this get open sourced!

out of curiosity, has the implementation been run through miri yet? Given the no_std, no dependency nature of the library, it should be pretty easy to do, and would be a great thing to mention in the Readme.

Cargo.toml Outdated
version = "0.1.0"
edition = "2021"
license = "MIT"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure if the missing authors field is intentional or not (just thought I'd point it out)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure what to put for that so I left it blank. I'll add authors = ["Microsoft"] I guess.

- Remove this section from the README
# StackFuture

This crate defines a `StackFuture` wrapper around futures that stores the wrapped future in space provided by the caller.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like GitHub markdown doesn't respect newlines between sentences, so if you're particular about formatting, double-check the rendered markdown looks how you expect.

@chris-oo
Copy link
Copy Markdown
Member

LGTM

@eholk
Copy link
Copy Markdown
Contributor Author

eholk commented Jul 21, 2022

out of curiosity, has the implementation been run through miri yet? Given the no_std, no dependency nature of the library, it should be pretty easy to do, and would be a great thing to mention in the Readme.

I've run miri manually and it works. I'm going to get CI set up pretty soon and I'll make sure miri runs as part of that. That's probably the right time to mention miri in the README. I'd like to make a stronger claim than just "Eric usually remembers to run miri before pushing changes."

@eholk eholk merged commit ae58939 into microsoft:main Jul 21, 2022
@eholk eholk deleted the initial-code-export branch July 21, 2022 19:23
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.

3 participants