Skip to content

Conversation

@talcia
Copy link

@talcia talcia commented Jun 10, 2021

No description provided.

.prettierrc.js Outdated
jsxBracketSameLine: false,
arrowParens: "always",
parser: "typescript",
endOfLine: "crlf",
Copy link
Member

Choose a reason for hiding this comment

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

I think we shouldn't change the default setting here. In most cases, this code will run on Linux machines so "lf" could be better

<div>
<Link href={Routes.ForgotPasswordPage()}>
<a>Forgot your password?</a>
<a href="/">Forgot your password?</a>
Copy link
Member

Choose a reason for hiding this comment

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

Does this link work correctly when you added another href for the a element?

}

const { hashedPassword, ...rest } = user
const { ...rest } = user
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't seem right:

  1. Why we are destructuring the user if we return the entire result anyway.
  2. The previous intention (to not include the password in the response) has been changed without a good reason.
    If this change is a result of a linter error, you should use the omit function from lodash or some equivalent. As a result, you will return the user but without the hashadPassword property.

Copy link
Author

Choose a reason for hiding this comment

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

This is a linter error about assigned value and never used it. By omit function you mean to disable
eslint for this line?

middleware: [
sessionMiddleware({
cookiePrefix: 'CodersCrewApp',
cookiePrefix: "CodersCrewApp",
Copy link
Member

Choose a reason for hiding this comment

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

Why we have double quotes here when we set Prettier to use single ones?

Copy link
Author

Choose a reason for hiding this comment

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

I don't know why but when I try to change to single quotes I get an error from eslint and prettier/lint change this to double quotes on save. So this is the reason.
I tired to fix it but it didn't work. Should I try more or leave this? I'm not good in lint :(

package.json Outdated
"eslint-config-airbnb-base": "14.2.1",
"eslint-config-prettier": "8.3.0",
"eslint-import-resolver-typescript": "2.4.0",
"eslint-plugin-import": "^2.20.1",
Copy link
Member

Choose a reason for hiding this comment

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

We should use pinned versions of libraries (without the ^)

@8bcb 8bcb temporarily deployed to coders-crew-prettier-sezexayiq June 16, 2021 15:04 Inactive
@8bcb 8bcb temporarily deployed to coders-crew-prettier-ddr6c7lcd June 17, 2021 18:48 Inactive
@8bcb 8bcb temporarily deployed to coders-crew-prettier-fjwbxyath June 17, 2021 18:50 Inactive
@talcia talcia requested a review from KonradSzwarc June 17, 2021 18:51
@8bcb 8bcb temporarily deployed to coders-crew-prettier-wd3y1dgwt June 22, 2021 22:18 Inactive
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.

4 participants