8000 Requesting Review · Issue #4 · Shaka-n/hanekawa · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Requesting Review #4

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.

Sign up for GitHub

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

Jump to bottom
Open
Shaka-n opened this issue May 19, 2023 · 1 comment
Open

Requesting Review #4

Shaka-n opened this issue May 19, 2023 · 1 comment

Comments

@Shaka-n
Copy link
Owner
Shaka-n commented May 19, 2023 •
edited
Loading

Hi @curtisault!

Let me know if you see anything horrendous. Most of my non-boilerplate code is in: https://github.com/Shaka-n/hanekawa/tree/main/lib/hanekawa
@curtisault
Copy link
curtisault commented Jun 5, 2023 •
edited
Loading

Apologies for the delayed review. A few things:

You have no authentication for this API. I would, at a minimum, at a hardcoded list of authorized emails or build a small registration capability. If you've deployed this somewhere, it maybe be open to bad actors for abuse if they ever discover this on the open web via Shodan or something.

lib/hanekawa/workers/movie_night.ex

:default is the default value for the queue. You may want to consider a different name for tracking purposes.

lib/hanekawa/mailer.ex

Doesn't do anything.... is it supposed to? Maybe it's just a 📌 for later use. Totally cool w/ that but wanted to point that out.

lib/hanekawa/movie_night.ex

I recommend the date field be utc_datetime. If it were up to me, all datetime fields would always use utc_datetime for an eternity until space travel becomes a thing for the regular human. Then we use whatever standard intragalactic standardization of time exists at that point.

You're using Phoenix 1.7.2, put up a live page buddy! Use ChatGPT to spit out a simple Tailwind view, add it as a component, and render it on some live page. You can add storybook as a dependency to help make this easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@curtisault @Shaka-n
0