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

REST Module #49

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

REST Module #49

wants to merge 6 commits into from

Conversation

hvalkerie19
Copy link
Contributor

REST Security Module (issue #11)

@houllette houllette linked an issue Feb 1, 2023 that may be closed by this pull request
@houllette houllette added new content New topics, lessons, or examples non-elixir content Distinguishes content additions that pertain to more general security practices labels Feb 1, 2023
@houllette houllette marked this pull request as draft February 1, 2023 23:00
@hvalkerie19
Copy link
Contributor Author

Section complete and ready for review

@hvalkerie19 hvalkerie19 marked this pull request as ready for review February 10, 2023 22:42
@houllette
Copy link
Collaborator

This is great - really nice work! Here are a few notes of feedback that I think should be incorporated before we move to merge this in.

Larger, more "ethereal" feedback:

  • The content (subjects covered, structure, etc.) is basically perfect, but I think the voice of the writing within this module as it stands may be a bit too formal and could use a once over to make it more casual in tone.
    • I think going over it with that lens may lead to removing some instances of "OWASP says" that's popped up here and there as well as making some parts a bit more concise.
  • None of the content currently refers to / incorporates Elixir: I think there's a great opportunity to outline how the Phoenix framework ties into some of the subsections you created in this document - which subsequently may lead to the creation of some Quiz/Examples.

Small Things:

  • There are a few typos in the form of non-capitalized proper nouns and such
  • Let's slot this in as the new Module 5 in the training, so this will require a few changes:
    1. Update the Module title on line 1
    2. Update the page navigation on the last line of the module so that the previous module is 4-graphql.livemd and just update the 5-elixir.livemd to read 6-elixir.livemd
      • Since we're slotting this into the middle of the ESCT, it will cause a cascading change of filenames and references to the other modules - so let's not worry about updating everything right now, we can fix that in a different PR

@houllette houllette marked this pull request as draft February 11, 2023 00:39
@houllette houllette changed the title Create 10-rest.livemd REST Module Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new content New topics, lessons, or examples non-elixir content Distinguishes content additions that pertain to more general security practices
Development

Successfully merging this pull request may close these issues.

REST Security Module
2 participants