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

Use the correct application directories for the platform #13

Merged
merged 3 commits into from
Oct 27, 2019
Merged

Use the correct application directories for the platform #13

merged 3 commits into from
Oct 27, 2019

Conversation

steven-joruk
Copy link
Contributor

Platform appropriate directories should be used, the dirs crate is already used so it's easy to do.

Also:

  • Replace let mut f = p; f.push(x) with let f = p.join(x)
  • Use println! for normal messages

* Replace `let mut f = p; f.push(x)` with `let f = p.join(x)`
* Use `println!` for normal messages
@steven-joruk
Copy link
Contributor Author

I've updated it to include a fix for #7 in a separate commit since it's the same section of code.

With the existing logic. re-generating a password file would lead to filite running without any feed back for the user, so it looked like it was hung while generating the hash.

I've changed it so no restart is required after a password is entered, and "Listening on port {}" is always shown to make it clear things are working.

@raftario
Copy link
Owner

Looks all good, will just need to test it once I'm able to do it!

@steven-joruk steven-joruk mentioned this pull request Oct 26, 2019
@steven-joruk
Copy link
Contributor Author

Btw I’d like to add unit tests but I’m not so familiar with actix. I looked at their guide but their examples are basic and didn’t include calls to routes which didn’t include extra arguments such as password hashes or identities. It’s be good to make them easy and intuitive to implement soon.

@raftario
Copy link
Owner

Yeah never really got unit tests for conplex routes either, I was planning to concentrate on having good integration tests instead. Would still be nice to have unit tests, I guess asking on the subreddit for example projects could be a good start?

@raftario
Copy link
Owner

I can also add you to the repo contributors if that makes development easier for you btw

Copy link
Owner

@raftario raftario left a comment

Choose a reason for hiding this comment

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

Really small and stupid stuff, I can change it myself if you prefer. Apart from those it's all good, will look at the 2nd one.

src/main.rs Show resolved Hide resolved
src/setup.rs Outdated Show resolved Hide resolved
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.

None yet

2 participants