-
Notifications
You must be signed in to change notification settings - Fork 125
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
add package to support metadata #1193
base: main
Are you sure you want to change the base?
add package to support metadata #1193
Conversation
Bencher Report
Click to view all benchmark results
|
Bencher Report
Click to view all benchmark results
|
Bencher Report
Click to view all benchmark results
|
hmm I'm not sure we want to have empty files to just to suppress warnings, that feels like the opposite direction see #1194 |
@plebhash, I’m unsure about removing the metadata from the root Cargo file. Another possible way to retain the metadata is by adding it to each crate within the workspace, but that seems excessive. I’m fine with either approach. |
I didn't notice you were assigned to #1192 feel free to modify this PR instead of #1194 but yeah, we can "transfer" the relevant fields from the workspace the main point is that those fields need to be removed from the workspace and we should not suppress the warnings by creating empty packages, that definitely feels like the wrong direction |
883eadb
to
0cbc1dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few nits:
- What do you think about adding
description
as well? documentation
is always linking to "https://github.com/stratum-mining/stratum", why not link to the specific crate docs on docs.rs?
@@ -1,7 +1,7 @@ | |||
[package] | |||
name = "interop-cpp" | |||
version = "0.1.0" | |||
authors = ["fi3 <email@email.org>"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need @Fi3 approval for those author changes..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..why do we actually need to change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will wait for @Fi3 confirmation on this.
description = "API for bridging SV1 miners to SV2 pools" | ||
license = "MIT OR Apache-2.0" | ||
documentation = "https://github.com/stratum-mining/stratum" | ||
license = "MIT + Apache-2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is or, not and
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get it, Can you elaborate?
|
closes #1192