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: set up pre-commit hooks (DEV-2393) #420

Merged
merged 12 commits into from Jun 28, 2023
Merged

Conversation

jnussbaum
Copy link
Collaborator

resolves DEV-2393

@jnussbaum jnussbaum self-assigned this Jun 28, 2023
@linear
Copy link

linear bot commented Jun 28, 2023

DEV-2393 DSP-TOOLS: set up pre-commit hooks

For certain tasks, it would be helpful to have pre-commit hooks that prevent bad stuff from being committed locally.

Especially formatting with black: If the CI tests fail only because of this, it's a waste of time: Better auto-format it before the commit actually happens.

@@ -1,4 +1,4 @@
artwork;;
artwork;;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove UTF-8 Byte Order Mark (made by one of the pre-commit hooks)

@@ -1,4 +1,4 @@
id,restype,label,ark,iri,created,permissions,file,file permissions,prop name,prop type,prop list,1_value,1_encoding,1_permissions,1_comment,2_value,2_encoding,2_permissions,2_comment,3_value,3_encoding,3_permissions,3_comment,4_value,4_encoding,4_permissions,4_comment, ,,,,,, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
id,restype,label,ark,iri,created,permissions,file,file permissions,prop name,prop type,prop list,1_value,1_encoding,1_permissions,1_comment,2_value,2_encoding,2_permissions,2_comment,3_value,3_encoding,3_permissions,3_comment,4_value,4_encoding,4_permissions,4_comment, ,,,,,, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove UTF-8 Byte Order Mark (made by one of the pre-commit hooks)

<a class=""salsah-link"" href=""IRI:document_thing_1:IRI"">document_thing_1</a>
<a class=""salsah-link"" href=""IRI:text_thing_1:IRI"">text_thing_1</a>
<a class=""salsah-link"" href=""IRI:zip_thing_1:IRI"">zip_thing_1</a>
<a class=""salsah-link"" href=""IRI:audio_thing_1:IRI"">audio_thing_1</a>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't know what the tool made here, but I compared the diff carefully, and there isn't a semantic change.

Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Choose a reason for hiding this comment

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

How long does this hook run? Personally I would be very much opposed to a workflow that makes committing take more than 5-10 seconds. But of course it's up to you in the end.

@jnussbaum
Copy link
Collaborator Author

It only takes ca. 3-5 seconds. I think I will collect some experience, and see if I'm going to like it or not.

@jnussbaum jnussbaum merged commit 9578181 into main Jun 28, 2023
4 checks passed
@jnussbaum jnussbaum deleted the wip/configure-pre-commit branch June 28, 2023 13:09
@daschbot daschbot mentioned this pull request Jun 28, 2023
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.

None yet

2 participants