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

Notes on README #17

Open
RobStallion opened this issue Dec 1, 2018 · 5 comments
Open

Notes on README #17

RobStallion opened this issue Dec 1, 2018 · 5 comments

Comments

@RobStallion
Copy link
Member

RobStallion commented Dec 1, 2018

@nelsonic This README is AWESOME!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! (I know this is not what issues are for but it really is amazing. Thanks so much)

I have gone through and made some points, and wanted to list them here. I can separate them out into different issues if needed and start working on them but just wanted to have them documented somewhere.

Prerequisites?

Basic understanding of Ecto (the module used to interface with databases in elixir/phoenix)

There is no link for this point (The previous points have links to learning repos). I have looked through the learn-phoenix-framework repo but couldn't find anything specific to just learning the basics of ecto (almost all the examples have something to do with ecto in them but nothing ecto specific). I was thinking that we could make a tutorial in the learn-phoenix repo that introduces that basics of ecto (migration/schema/repo) (I would be up for making this just to be clear 👍)

Time Requirement?

Simply reading ("skimming") through this example will only take 15 minutes.
Following the examples on your computer (to fully understand it) will take around 1 hour

Personally, I'm not a fan of the time thing as I feel it can make people feel down if they take longer. Just my opinion though.

How

Before the

mix ecto.migrate

command, we need to edit the migration file. It needs the following line added to it

create(unique_index(:users, [:email_hash]))

This missing line was causing tests to fail later on in the example.

3. Define The 6 Functions

3.4 Hash Email Address

These tests depend on the :secret_key_base. They were failing for me following the example because mine was different. Not sure how best to resolve this. Could we use a pattern match similar to the decrypt function.

4 Create EncryptedField Custom Ecto Type

I know you suggest users write their own tests at this point so not really a problem but just thought I would mention this. The tests in test/lib/encrypted_field_test.exs do not test the load/1 function (only the load/2). This function may not exist for people following the readme. (Again, not an issue, just mentioning it 👍)

5. Use EncryptedField Ecto Type in User Schema

alias Encryption.EncryptedField

Also need to add User as an alias at this point as it is used in the changeset function.

8. Create PasswordField Ecto Type for Hashing Email Address

def verify_pass(password, stored_hash) do
Argon2.verify_pass(password, stored_hash)
end

This functions name has been changed in error. Should be verify_password. Caused tests in password_field_tests.exs to fail.

10.2 Re-order :email_hash Field in User Schema

users schema never shows adding a :password field during the example

field :password, :binary, virtual: true # virtual means "don't persist"`

This doesn't break any tests or anything. Just something I noticed. Was it added for the user interface mentioned in the conclusion?

@nelsonic
Copy link
Member

nelsonic commented Dec 1, 2018

@RobStallion thanks for the feedback. I agree with most of your points and value your insight!
The approximate time required to go through the example/tutorial is only a guide. In some cases it will take people less time (because they have more Elixir/Phoenix experience) and in others it will take more because they are having to learn Phoenix and Encryption fundamentals simultaneously.

I really wish we had a way of A/B testing this on enough "subjects" to collect the feedback/data on this.
The sentiment that "_if it takes me longer than an hour I must be 'slow'..."
is something I have felt as a dyslexic "slow reader", but it hasn't discouraged me.
if anything I use it as a "benchmark" to strive for and speed up my reading/comprehension.
But I know that other people do not approach this in the same way.

Please make any/all changes you see fit and open a PR.
I trust you to approach this with a "beginners mind" and make the appropriate changes. 👍

As for a more "beginners" ecto tutorial, have you seen: https://github.com/dwyl/phoenix-chat-example ?

@RobStallion
Copy link
Member Author

@nelsonic I haven't gone through the chat-example above myself but I have just had a quick read through it now and I think it looks like a great place to start out.

I understand your point about the timing. I think it is just different strokes for different folks. Some may feel discouraged and some may feel spurred on. It wasn't meant to be a big point however, the quality of the tutorial/example is much more important than the time estimation (and this README was great ❤️)

@nelsonic
Copy link
Member

nelsonic commented Dec 2, 2018

@RobStallion I'm stoked that you mentioned the "estimated learning time". 🥇
I feel that learning pace is a "super power" we can & should all strive develop.
The faster you learn (anything!) the faster you can reach your goals (in life).

I would sooner hire or work with someone who is a "fast learner" than someone who already knows how to do something, fast learners can acquire (new) skills during the project whereas people who "already know what they need" are often reluctant to learn new skills or "drag their feet" ... 🐌 😕

Without a reference for the time something can take you to learn, you will take days/weeks/months to learn something instead of minutes/hours. Learning something eventually is of little use in the fast-moving world of tech. By the time you acquire the skills everything has changed and your knowledge is "obsolete" (or at least advances in other tech have eclipsed your "competitive advantage"). ☎️

Obviously, this really only applies to JS-world where there's a new framework/flavour of the week every week, but you get the idea; the goal is to coach yourself to learn faster. 🚀 😉

@RobStallion
Copy link
Member Author

RobStallion commented Dec 2, 2018

Opening a pr with updates for most of the points I raised above. @nelsonic can you give me access to do so 👍

I am going to do the following point on a separate branch as it is a little more complex than the other changes

3.4 Hash Email Address

These tests depend on the :secret_key_base. They were failing for me following the example because mine was different. Not sure how best to resolve this. Could we use a pattern match similar to the decrypt function.

@nelsonic
Copy link
Member

nelsonic commented Dec 2, 2018

@RobStallion you should already have write-access. you just need to accept it. 😉
Look forward to seeing your PR. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants