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

gigasecond: Update dependency to conform to usage guidelines #436

Merged
merged 1 commit into from
Mar 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions exercises/gigasecond/.meta/hints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If you're unsure what operations you can perform on `DateTime<Utc>` take a look at the [chrono crate](https://docs.rs/chrono/0.4.0/chrono/) which is listed as a dependency in the `Cargo.toml` file for this exercise.
Copy link
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 if there was a specific format for .meta/hints.md files. I looked at a couple exercism repos on Github but didn't see any. Let me know if this is alright.

Copy link
Member

Choose a reason for hiding this comment

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

There's no specific format necessary there; it's all just human-readable text.

You will, however, need to regenerate the README for this to pass CI:

  • Get the configlet tool. On mac/linux you can just use the bin/fetch-configlet script; on Windows, you have to find it and install it manually.
  • Run bin/configlet generate . --only gigasecond to regenerate the readme
  • Commit the new README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the guidance. I just updated the README.

3 changes: 3 additions & 0 deletions exercises/gigasecond/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Calculate the moment when someone has lived for 10^9 seconds.

A gigasecond is 10^9 (1,000,000,000) seconds.

If you're unsure what operations you can perform on `DateTime<Utc>` take a look at the [chrono crate](https://docs.rs/chrono/0.4.0/chrono/) which is listed as a dependency in the `Cargo.toml` file for this exercise.
Copy link
Contributor Author

@cbzehner cbzehner Mar 1, 2018

Choose a reason for hiding this comment

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

I'm still a bit concerned this isn't a clear enough hint to walk users to adding Duration as an import from the chrono crate. When I look at the crate documentation on Duration:

Chrono currently uses the time::Duration type from the time crate to represent the magnitude of a time span...

It implies to me as a beginner in Rust that I should be doing the same thing chrono does and adding time to my Cargo.toml so I can also use the time::Duration type.

Duration doesn't appear in reexports and the function most people end up using for this exercise Duration::from isn't apparent anywhere on the page.

Do y'all have any suggestions on how to help this hint provide a bit more structured guidance for people new to the Cargo ecosystem?

Copy link
Member

Choose a reason for hiding this comment

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

I'm still a bit concerned this isn't a clear enough hint to walk users to adding Duration as an import from the chrono crate.

I see what you mean because there's a big dedicated section about durations being from time and being unclear on what relationship this has with https://docs.rs/chrono/0.4.0/chrono/struct.Duration.html.

I don't have any definitive answers, but how about some ideas?



## Rust Installation

Refer to the [exercism help page][help-page] for Rust installation and learning
Expand Down
2 changes: 1 addition & 1 deletion exercises/gigasecond/example.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
extern crate chrono;
use chrono::*;
use chrono::{DateTime, Duration, Utc};

pub fn after(start: DateTime<Utc>) -> DateTime<Utc> {
start + Duration::seconds(1_000_000_000)
Expand Down
14 changes: 7 additions & 7 deletions exercises/gigasecond/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
extern crate chrono;
use chrono::*;
Copy link
Member

Choose a reason for hiding this comment

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

Agree that we should remove the glob import as bad style, but I think we can do better. What about:

use chrono::{DateTime, Duration};

This way we don't have to mess with a second dependency or globs.

Copy link
Member

Choose a reason for hiding this comment

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

This applies both to the lib stub and the example.

// Returns a Utc DateTime one billion seconds after start.
pub fn after(start: DateTime<Utc>) -> DateTime<Utc> {
unimplemented!()
}
extern crate chrono;
use chrono::{DateTime, Utc};

// Returns a Utc DateTime one billion seconds after start.
pub fn after(start: DateTime<Utc>) -> DateTime<Utc> {
unimplemented!("What time is a gigasecond later than {}", start);
}
2 changes: 1 addition & 1 deletion exercises/gigasecond/tests/gigasecond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ extern crate gigasecond;
* In order to use the crate, your solution will need to start with the two following lines
*/
extern crate chrono;
use chrono::*;
use chrono::{TimeZone, Utc};

#[test]
fn test_date() {
Expand Down