Skip to content
This repository was archived by the owner on Apr 19, 2021. It is now read-only.

Workflow enhancements #707

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Workflow enhancements #707

wants to merge 5 commits into from

Conversation

nisarhassan12
Copy link
Contributor

@nisarhassan12 nisarhassan12 commented Aug 4, 2020

This PR is aimed towards enhancing the overall code quality of the site.

  • adds pre-commit hook via husky to enforce code quality the hook runs tasks like format, 'test` (which intern runs lint task).
  • enable format on save for the workspace.

Edit

I have removed the format task from the pre-commit hook as format on saving is enabled with prettier extension being installed.

@nisarhassan12 nisarhassan12 force-pushed the workflow-enhancements branch from 77a4847 to 4be2966 Compare August 4, 2020 04:36
Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Wow, many thanks for adding a husky pre-commit hook and installing Prettier. ✨ This makes the project feel much cleaner and professional.

Unfortunately, this PR is a bit too big for me to seriously review it now (touches almost all blog posts and code files, and there are a few surprising changes mixed in like a new yarn.lock).

Could you please split up the work? (And maybe do that after some of your other PRs are merged, because otherwise you're going to make conflicting changes on the same files.)

E.g. maybe first install Prettier, and the pre-commit hook, without refactoring the entire codebase? Then, after that's merged, we can see about refactoring reasonable chunks of the codebase carefully while keeping PRs at reviewable size?

@@ -12,21 +12,23 @@ vscode:
- [email protected]:7hF8G3VtB+HNearI5Zw+NA==
- [email protected]:YnjK47pXScU3DMFfQzkkOw==

Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary extra empty line.

Suggested change

@nisarhassan12
Copy link
Contributor Author

Could you please split up the work? (And maybe do that after some of your other PRs are merged, because otherwise you're going to make conflicting changes on the same files.)

E.g. maybe first install Prettier, and the pre-commit hook, without refactoring the entire codebase? Then, after that's merged, we can see about refactoring reasonable chunks of the codebase carefully while keeping PRs at reviewable size?

@jankeromnes Many Thanks. yeah splitting the work and waiting for other PRs to be merged would be better for everyone. Currently, this PR is really quite hard to review. Sorry, I should have thought about this before.

I will make small PRs once my others PRs are merged.

@ghuntley ghuntley self-requested a review February 17, 2021 06:02
Copy link
Contributor

@ghuntley ghuntley left a comment

Choose a reason for hiding this comment

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

Agreed that there's value in this pull-request but too much was changed. Needs to be a smaller increment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants