-
Notifications
You must be signed in to change notification settings - Fork 13
misc changes #7
misc changes #7
Conversation
Signed-off-by: gibix <[email protected]>
Updated cargo to 0.27 with apis Add quicli to manage cli interafce Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
cardoe
left a comment
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.
Some of the updates look good. Need a few small changes. I'll likely cherry-pick in some commits and let you update from there.
| - nightly | ||
| - beta | ||
| - stable | ||
| - 1.17.0 # minimum supported version (due to unicode-bidi breakage) |
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?
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.
now 1.17 is nightly
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.
No. That's a stable release.
Cargo.toml
Outdated
| [package] | ||
| name = "cargo-ebuild" | ||
| version = "0.1.6-pre" | ||
| version = "0.1.7" |
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 use cargo-release so just leave this be.
Cargo.toml
Outdated
| [badges] | ||
| travis-ci = { repository = "cardoe/cargo-ebuild" } | ||
| [badges.travis-ci] | ||
| repository = "cardoe/cargo-bitbake" |
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 can't be right. I think I have an idea where you copied this from... ;)
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.
yep
src/main.rs
Outdated
| .unwrap_or_else(|| String::from(package.name())); | ||
| let desc = metadata.description.as_ref().cloned().unwrap(); | ||
| // TODO: now package.name is InternedString | ||
| // .unwrap_or_else(|| String::from(package.name())); |
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.
Yeah we'll need to resolve this prior to merge.
src/main.rs
Outdated
| .create(true) | ||
| .truncate(true) | ||
| .open(&ebuild_path) | ||
| .unwrap(); |
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 wouldn't want this to panic when you failed to open the file. Returning an error saying it couldn't create the file would be nicer.
src/main.rs
Outdated
| crates = crates.join(""), | ||
| cargo_ebuild_ver = env!("CARGO_PKG_VERSION"), | ||
| this_year = 1900 + time::now().tm_year, | ||
| ).expect("Error during ebuild file writing"); |
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.
Again this is going to panic instead of returning a nice error to the user.
|
with the last two commit there is a bit of refactoring and better error handling |
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
better errors with failure better cli support for env debut small fix Signed-off-by: gibix <[email protected]>
Signed-off-by: gibix <[email protected]>
|
Signed-off-by: gibix <[email protected]>
|
@cardoe nothing will never compile for 1.17. I understand the broken unicode-bidi release but why we need an expicit test on 1.17? |
cardoe
left a comment
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.
So some of the comment messages are a bit wonky. I think we need to work on cleaning up the history a bit. If you need some help with this ping me on IRC.
e.g a subject of "Update copyright" does the update to cargo 0.27 and then there are commits later on that revert part of those changes that should be squashed together.
|
This is a single, quite big, refactor change. Me can squash it with a better message and changelog. Something has been rewritten/reverted. |
call_main_without_stdin) and new cargo's apis