Conversation
xsharonhe
left a comment
xsharonhe
left a comment
There was a problem hiding this comment.
good changes overall! small comments - will approve just prefer to have some of the questions answered first before approving. also in the future, could we have the PR title be more descriptive? eg. cleanup / refactor of website (misc is kinda vague)
will check locally afterwards
| @@ -0,0 +1,10 @@ | |||
| // export function capitalizeWords(str) { | |||
There was a problem hiding this comment.
oh nope. I thought I could use it and then realized it wouldn't work. I left it in case I thought of a way to use it later
| Home: "", | ||
| "About Us": "about", | ||
| Resources: "resources", | ||
| Events: "events", | ||
| Community: "community", |
There was a problem hiding this comment.
nit: almost prefer for consistency that the keys all be strings - would this affect anything?
There was a problem hiding this comment.
do you think there is a way to use RESOURCES as the key names here? seems redundant to have both
|
|
||
| { | ||
| /* TODO: Refactor into object? paths must be same as linked pages*/ | ||
| /* @TODO: Refactor into object? paths must be same as linked pages */ |
There was a problem hiding this comment.
i think we can remove the redo comment here yes
| {RESOURCES.map((resource) => ( | ||
| <Resource key={resource}> | ||
| <Link href={`/${PATHS[index]}`}> | ||
| <Link href={`/${PATH_NAMES[resource]}`}> |
There was a problem hiding this comment.
I would pull this out into a variable so it isn't stated twice for repetition now that I think about it
| }, []); | ||
|
|
||
| const handleChange = (searchValue, tagValues, releaseValues) => { | ||
| // console.log([searchValue, tagValues, releaseValues]); |
There was a problem hiding this comment.
we can remove this altogether
| - tags = tags to filter | ||
| */ | ||
| export default async (req, res) => { | ||
| // console.log("tt", req.query); |
There was a problem hiding this comment.
same here please remove console statements altogether
| export const authors = ${getAuthors()}; | ||
| export const opportunities = ${getOpportunities()}; | ||
| const postsWithoutId = ${getProjects()}; | ||
| export const posts = postsWithoutId.map(post => { |
There was a problem hiding this comment.
Math.random() would assign a random id to this right? could we also add maybe something that would tell us what post it is like project name or something so? not too sure if it matters but might be more descriptive than a random number
Ah, alright. I'll also try and make fixes and commit again before that |
| try { | ||
| postPaths = await prisma.post.findMany(); | ||
| // @P_ERR | ||
| // postPaths = await prisma.post.findMany(); |
There was a problem hiding this comment.
what is the specific error here? this is not using our database but relying on our cache so just want to be sure i forgot to add this in earlier
There was a problem hiding this comment.
im sure its because i keep updating things like vulnerabilities but just curious if you could screenshot or smth
There was a problem hiding this comment.
Yep, here's my browser's screen,
https://gyazo.com/dd31a8773c9c771cf60c427477847645
This is the terminal's,
https://gyazo.com/b9591bcecad01bf3239332ca6bce2fb3
https://gyazo.com/668c1b53fadd231d0583e32020b9c1f1
There was a problem hiding this comment.
oh this is because you don't have the .env file - I have some time to dig it up tmrw
2 participants
just trying to get familiar.
Changes:
Idk, random stuff. Could be bad changes