Skip to content

Static site#2

Open
sreynen wants to merge 8 commits into
mainfrom
start-static
Open

Static site#2
sreynen wants to merge 8 commits into
mainfrom
start-static

Conversation

@sreynen

@sreynen sreynen commented Mar 11, 2020

Copy link
Copy Markdown
Contributor

This is on top of #1, just adding HTML and JS files.

@schuyler1d schuyler1d left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So I think I get the general idea here, now that I've seen both PRs.

Sorry comments are distributed between both -- but maybe consider the comments as the questions of someone coming to the codebase for the first time, unfamiliar with the tech (a likely scenario for us).

So I think my toplines (for both PRs) are:

  • document the security model -- (it is clearer with the static page content, but please put it in the readme)

  • switch to HMAC for the key construction/validation

  • Dont' call it 'Static Site' -- that caused a lot of confusion for me as to what I was trying to evaluate.

  • Fail harder/faster on missing campaign/source if they are expected to be required.

  • Maybe say a little about pywell -- and its usecases -- and a quick link or documentation on how to run a dev environment locally (to see the web part work).

  • How do we construct the keys -- is that in this repo as well? If so, then document that in the readme -- what command do I run to create a key?

Comment thread README.md Outdated
Comment thread validate_key.py
Comment thread export_rsvps.py
@schuyler1d

Copy link
Copy Markdown

Also, ideally, add a small test that just creates and validates a valid key, and then confirms that an invalid one does indeed fail.

@sreynen sreynen requested a review from schuyler1d April 3, 2020 14:07
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.

2 participants