search

utmmcss / mcss-website-frontend

MCSS Website Frontend
https://mcss.club
MIT License
0 stars 1 forks source link

Dev- Adding linting and type checks to pre-push and github actions #11

Closed TheFatPanda97 closed 3 years ago

TheFatPanda97 commented 3 years ago

Ticket

Ticket Link

What is changed / added

nigow commented 3 years ago

@TheFatPanda97 can you clean up the commits? I'm afraid it's hard to see what each commit does.

AaronCGoidel commented 3 years ago

@TheFatPanda97 can you clean up the commits? I'm afraid it's hard to see what each commit does.

I agree. I'd be happy squashing these when we merge, instead though.

TheFatPanda97 commented 3 years ago

@TheFatPanda97 can you clean up the commits? I'm afraid it's hard to see what each commit does.

I agree. I'd be happy squashing these when we merge, instead though.

Yep, I think squash and merging would be good enough.

TheFatPanda97 commented 3 years ago

Dev - Adding linting rule to GitHub Actions and husky

nigow commented 3 years ago

It might boil down to personal taste, but I see each commit when I see a PR (because it might include unmerged commits from a different branch). So I would prefer PRs git squashed to smaller commits with good commit messages before review, but we didn't decide on this rule so we can always discuss later.

TheFatPanda97 commented 3 years ago

It might boil down to personal taste, but I see each commit when I see a PR (because it might include unmerged commits from a different branch). So I would prefer PRs git squashed to smaller commits with good commit messages before review, but we didn't decide on this rule so we can always discuss later.

Hmm I see, I think it's just a different type of review style. For me, I would just look at the final changes rather than the commit since there may be a lot of testing in the commits. Since we are doing a squashing and merging at the end, the master branch would only have 1 commit. From my experience, it is unnecessary to squash the commits before the PR, it is sufficient to just review the final changes.

I'll merge this PR for now since linting checks is essential for all the other PRs. We can continue this discussion in another issue if needed.