lukemathwalker / build-your-own-jira-with-rust Goto Github PK

See rust-lang/rust-analyzer#4177.

Just like C headers, include! can be problematic for IDEs because there's no way to offer a good experience in the presence of code like this.

I don't know if it works with full project, but an alternative is to produce something like:

#[path = "koans/00_greetings"]
mod greetings {
    #[path = "00_greetings.rs"]
    mod greetings;
}

instead of

include!("koans/00_greetings/00_greetings.rs");

• In 05_type_as_constraints,rs there is

    /// Has it been saved yet? Is it safe to unwrap whose `Option`s?
    /// That is unnecessary cognitive load and lead to errors down the line,

but maybe it should be

    /// Has it been saved yet? Is it safe to unwrap those `Option`s?
    /// That is unnecessary cognitive load and leads to errors down the line,

• In 06_results, the example of using Err(ValidationError(... does not use the constructor Err(ValidationError::new("Some error message").

Also, isn't

    impl std::fmt::Display for ValidationError {
        fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
            write!(f, "{}", self.0)
        }
    }

is supposed to be something like:

    impl std::fmt::Display for ValidationError {
        fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
            write!(f, "{}", self.0)?;
            Ok(())
        }
    }

Maybe write! has changed, since clippy isn't saying anything.

First of all, this was a great introduction to Rust. It goes through many topics in a logical and easy to follow order, the in-code explanations are great, and the koans tool is nice for managing the tasks. Great work! 😄

Here's my feedback from running through it, in no particular order:

• I'd put the requirements at the top of the README, rather than after the setup.

• Providing a link to https://github.com/rust-analyzer/rust-analyzer or https://intellij-rust.github.io/ for users to setup syntax hightlighting and auto-completion may be nice.

• The Koans CLI color codes don't render nicely on Windows. This isn't a huge problem, but makes the output a little more awkward to read. I tested this on Powershell and in Git Bash through https://github.com/microsoft/terminal and neither worked. (I didn't test WSL).

  Example

       Jamie in build-your-own-jira-with-rust on  my-solution [?] via 🦀 v1.42.0
       ❯ koans --path jira-wip
   
   
               ←[3mEternity lies ahead of us, and behind. Your path is not yet finished. 🍂←[0m
   
       Do you want to open the next koan, (00) greetings - (00) greetings? [y/n]
  

• It seems that the Koans CLI swallows the test output and prints it at the end (if there's a failure). This does mean that the first run looks as if the program has hung (I actually killed it once or twice before looking at the source to see what it was doing). Printing some output to the effect of "Running tests..." may help flag users as to why its taking a bit longer.

• Related to the previous point, the test/build output from Koans has the colors stripped which makes it a little harder to grok.

• There's a small disconnect between the CLI 'opening a koan' and what I need to do next. I expected a file to open or a path to be printed. After the first one, I understood what was going on though.

• There's a missing link here https://github.com/LukeMathWalker/build-your-own-jira-with-rust/blob/master/jira-wip/src/koans/01_ticket/05_ownership.rs#L71

• In 01_ticket/06_traits we encounter match for the first time. Perhaps linking to the documentation for this would be helpful. (https://doc.rust-lang.org/book/ch06-02-match.html or https://doc.rust-lang.org/book/ch18-03-pattern-syntax.html)

• There's commented out code https://github.com/LukeMathWalker/build-your-own-jira-with-rust/blob/master/jira-wip/src/koans/02_ticket_store/01_store.rs#L18 that doesn't appear to serve a purpose?

• Is it intentional that https://github.com/LukeMathWalker/build-your-own-jira-with-rust/blob/master/jira-wip/src/koans/02_ticket_store/01_store.rs#L48 initially returns TicketStore?

• In 02_store/08_delete_and_update I had to add an artificial delay (as in, a thread::sleep()) into update(...) to ensure that the updated timestamp is different from the created timestamp. Otherwise the timestamps were exactly the same.

• The test description_cannot_be_longer_than_3000_chars is flakey since we generate a string between 3000 and 10000 characters so you could get unlucky and get 3000 exactly which passes the validation and fails the test. I am, apparently, unlucky 😅 (I can raise a quick PR for this one).

            // You should be seeing this error when trying to run this koan:
            //
            // error[E0616]: field `description` of struct `path_to_enlightenment::visibility::ticket::Ticket` is private
            // --> jira-wip/src/koans/01_ticket/04_visibility.rs:99:25
            //    |
            // 99 |              assert_eq!(ticket.description, "A description");
            //    |                         ^^^^^^^^^^^^^^^^^^
            //
            // If that is the case, comment the next line out and move on to the next koan!
            assert_eq!(ticket.description, "A description");

The above is all true, but I see this error without having Add the necessary pub modifiers in the code above... i.e. I made no modifications and it's instructing me to go to the next koan?

Also maybe be a bit more explicit on this step:
Once you have verified that it does not compile, =>
Once you have verified that the below does not compile,

Hello,

I have here to write a get so I did this :

pub fn get(&self, id: &u32) -> &Ticket {
        self.data.get(id)
     }

But that gives me a Option which we did not learn.
So is there another way to make this part work ?

hello,

Im now looking at this part

/// So far we have allowed any string as a valid title and description.
/// That's not what would happen in JIRA: we wouldn't allow tickets with an empty title,
/// for example.
/// Both title and description would also have length limitations: the Divine Comedy probably
/// shouldn't be allowed as a ticket description.
///
/// We want to define a function that takes in a title, a description and a status and
/// performs validation: it panics if validation fails, it returns a `Ticket` if validation
/// succeeds.
///
/// We will learn a better way to handle recoverable errors such as this one further along,
/// but let's rely on panic for the time being.
fn create_ticket(title: String, description: String, status: Status) -> Ticket {
                                                                             todo!()
                                                                                    }
                                                                                    ````

What am I suppose to do ?

and how do I know what a valid title and description is ?

If there is a .01_ticket.rs.swp file then this gets added to the list of koans as a second instance of the current koan. This means that when tests pass it asks if you'd like to move onto the next koan but repeats the current one instead.

error[E0428]: the name `greetings` is defined multiple times
 --> jira-wip/src/path_to_enlightenment.rs:6:1
  |
3 | mod greetings;
  | -------------- previous definition of the module `greetings` here
...
6 | mod greetings;
  | ^^^^^^^^^^^^^^ `greetings` redefined here
  |
  = note: `greetings` must be defined only once in the type namespace of this module

For more information about this error, try `rustc --explain E0428`.
error: could not compile `jira-wip` (bin "jira-wip" test) due to previous error

I don't know where this needs to go, but if

fn run_tests(manifest_path: &Path, filter: Option<&str>) -> TestOutcome {
    let mut args: Vec<OsString> = vec![
        "test".into(),
        "--manifest-path".into(),
        manifest_path.into(),
        "-q".into(),
        "--color=always".into()
    ];
...

Then we get colored output. It is not the best ever, but it is something. I don't know where this actually needs to go with this yansi stuff.. So here's the suggestion.

> koans --path jira-wip/


Running tests...

        🚀 (00) greetings - (00) greetings
        🚀 (01) ticket - (01) ticket
        🚀 (01) ticket - (02) status
        🚀 (01) ticket - (03) validation
        🚀 (01) ticket - (04) visibility
        🚀 (01) ticket - (05) ownership
        🚀 (01) ticket - (06) traits
        🚀 (01) ticket - (07) derive
        🚀 (01) ticket - (08) recap
        🚀 (02) ticket_store - (01) store
        🚀 (02) ticket_store - (02) option
        🚀 (02) ticket_store - (03) id_generation
        🚀 (02) ticket_store - (04) metadata
        🚀 (02) ticket_store - (05) type_as_constraints
        🚀 (02) ticket_store - (06) result
        🚀 (02) ticket_store - (07) vec
        ❌ (02) ticket_store - (08) delete_and_update

        Meditate on your approach and return. Mountains are merely mountains.




running 15 tests
............F..
failures:

---- path_to_enlightenment::delete_and_update::tests::update_works stdout ----
thread 'path_to_enlightenment::delete_and_update::tests::update_works' panicked at 'assertion failed: `(left != right)`
  left: `2020-04-16T15:28:58.708731250Z`,
 right: `2020-04-16T15:28:58.708731250Z`', jira-wip/src/koans/02_ticket_store/08_delete_and_update.rs:272:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    path_to_enlightenment::delete_and_update::tests::update_works

test result: FAILED. 14 passed; 1 failed; 0 ignored; 0 measured; 41 filtered out

warning: unused imports: `Status`, `Ticket`, `create_ticket`
  --> jira-wip/src/koans/01_ticket/04_visibility.rs:62:29
   |
62 |         use super::ticket::{create_ticket, Status, Ticket};
   |                             ^^^^^^^^^^^^^  ^^^^^^  ^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

error: test failed, to rerun pass '--bin jira-wip'

Code

        /// If the `id` passed in matches a ticket in the store, we return the deleted ticket
        /// with some additional metadata.
        /// If it doesn't, we return `None`.
        pub fn delete(&mut self, id: &TicketId) -> Option<DeletedTicket> {
            self.data.remove(id).and_then(|ticket| Some(DeletedTicket {
                ticket,
                deleted_at: Utc::now(),
            }))
        }

I think that is the same code as 09 but the code does not work.