Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

Add yarn commands #21

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add yarn commands #21

wants to merge 3 commits into from

Conversation

mironiasty
Copy link
Contributor

@mironiasty mironiasty commented May 27, 2024

EX-527

Add simple commands to automatically format/lint iOS code

@mironiasty mironiasty requested review from graszka22 and karkakol and removed request for graszka22 June 3, 2024 12:51
@mironiasty mironiasty marked this pull request as ready for review June 3, 2024 12:51
Copy link
Member

@karkakol karkakol left a comment

Choose a reason for hiding this comment

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

You should also update .githooks/pre-commit.sh script

@mironiasty
Copy link
Contributor Author

You should also update .githooks/pre-commit.sh script

What exactly do you want to update in pre-commit.sh?

@karkakol
Copy link
Member

You should also update .githooks/pre-commit.sh script

What exactly do you want to update in pre-commit.sh?

It should use yarn commands instead of swift-formater ... etc.

@mironiasty
Copy link
Contributor Author

In that case, I'm not sure about it. Commands in pre-commit are designed to run only on modified files (swift-format format -i "${line}"), but commands in yarn are designed to run on all files (swift-format lint -r -s ./**/**/*.swift). So I would have to create second set of commands in yarn just for git hook. TBH I think it would make more sense to only rely on GH Actions and remove hooks in that case.

@karkakol
Copy link
Member

In that case, I'm not sure about it. Commands in pre-commit are designed to run only on modified files (swift-format format -i "${line}"), but commands in yarn are designed to run on all files (swift-format lint -r -s ./**/**/*.swift). So I would have to create second set of commands in yarn just for git hook. TBH I think it would make more sense to only rely on GH Actions and remove hooks in that case.

Imho if we require some checks on code like format or lint it should be checked also in pre-commit, otherwise i do not see a reason for it to be present. It is annoying if pre-commit passes and then github action fails. In this case we should delete pre-commit script or update to be the same as lint/format checks.

@mironiasty
Copy link
Contributor Author

I guess that is the point of pre-commit script - to fail if code is not 'commitable'. Not all eslint rules can be autofixed. Also there are many rules on precommit hook that can be added, that will fail commit (checking version of tools used as an example)

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.

2 participants