Skip to content
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.

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

chore(infra): only lint staged files #153

Merged
merged 6 commits into from
Apr 28, 2022
Merged

chore(infra): only lint staged files #153

merged 6 commits into from
Apr 28, 2022

Conversation

petermuessig
Copy link
Member

No description provided.

@marianfoo
Copy link
Member

just had to change gh workflow
i´m not sure how yarn workspaces run build works, is it alway alphabetical? crawl -> ui
if we can make that sure we can run in workflow just this one command to run crawl and build ui

@marianfoo marianfoo self-requested a review April 24, 2022 18:59
@petermuessig
Copy link
Member Author

AFAICS, it runs the build in the alphabetical order. Due to the pattern packages/* it uses the order how the pattern resolves the projects. If you want a different order, you can specify the workspaces directly via packages/ui and packages/crawler in the workspaces configuration of the root package.json.

@marianfoo
Copy link
Member

Just for me to understand:
during commit prettier is executed?
image

on push what is executed?
i see → No staged files found here, but on push there are never staged files, right?
image

@petermuessig
Copy link
Member Author

That's right, @marianfoo - I missed to adopt to move this to the pre-commit phase. The pre-push only made sense because of the full lint run.

@petermuessig
Copy link
Member Author

Only the lint:commit makes sense for the pre-push hook as we want to only validate the last commit message which is pushed. We can also agree to ensure the commit message validation for all intermediate commits as well?

@marianfoo
Copy link
Member

Only the lint:commit makes sense for the pre-push hook as we want to only validate the last commit message which is pushed. We can also agree to ensure the commit message validation for all intermediate commits as well?

Because of the merge commit it is enough to lint the last commit message.
It´s easier to commit then change last commit message.

@petermuessig
Copy link
Member Author

Change is good to go - can finally discuss it tomorrow morning...

@marianfoo
Copy link
Member

will do!

Copy link
Member

@marianfoo marianfoo left a comment

Choose a reason for hiding this comment

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

LGTM!

"lint": "yarn workspaces run lint",
"lint:staged": "yarn workspaces run lint:staged",
"hooks:pre-commit": "npm-run-all --sequential prettier:staged lint:staged",
"hooks:pre-push": "npm-run-all --sequential lint:commit"
Copy link
Member

Choose a reason for hiding this comment

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

this will run pretty-quick --staged for all workspaces
it seems this is not divided by workspace and will just prettier all files, not just for ui/crawler
so the command with the same result is then executed twice here
the previous command was already ok
image

@marianfoo marianfoo merged commit 1364292 into main Apr 28, 2022
@marianfoo marianfoo deleted the lint-staged-only branch May 13, 2022 07:37
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.

2 participants