kpi-web-guild / django-girls-blog-OlenaEfymenko

django-girls-blog-OlenaEfymenko created by GitHub Classroom
MIT License
1 stars 0 forks source link

Integrate pre-commit tool into project #15

Closed OlenaYefymenko closed 1 year ago

OlenaYefymenko commented 1 year ago

Settings the pre-commit tool will enable files inspection before committing. The check includes the following:

More details at https://pre-commit.com/hooks.html

Resolves #12

webknjaz commented 1 year ago

@OlenaYefymenko also, Git allows working on multiple things simultaneously. To practice this, start working on #13 and send another PR in parallel.

OlenaYefymenko commented 1 year ago

Thank you for providing such detailed feedback. I will take all aspects into consideration and make the necessary fixes.

OlenaYefymenko commented 1 year ago

Description

The pull request (PR) introduces configuration settings for pre-commit. Using Git hook scripts is a practical way to catch simple issues early on, before submitting code for review. These scripts automatically run on every commit and can flag problems such as missing semicolons, trailing whitespace, and debug statements. This approach saves time for code reviewers, who can then focus on higher-level aspects of a change instead of wasting time on minor style issues.

Resolves #12

Changed Files

.pre-commit-config.yaml

After installing the new pre-commit hooks, you can verify that they are working correctly by running the command pre-commit run --all-files

Reviewers

@webknjaz

webknjaz commented 1 year ago

@OlenaYefymenko don't post that as a comment, edit the PR title and description.

webknjaz commented 1 year ago

Also, there's review request button in the sidebar and some other places, use those. Comments are for discussions, they don't update those fields automatically.

webknjaz commented 1 year ago

Also, update the commit message with the new details, you need to use Git for that.

webknjaz commented 1 year ago

Pre-commit hooks enable checking automatically the Git repository before each commit

Only if one integrates the tool into git or runs it manually. This PR does not make it automatic or integrated with Git in any way.

OlenaYefymenko commented 1 year ago

Only if one integrates the tool into git or runs it manually. This PR does not make it automatic or integrated with Git in any way

@webknjaz thank you for feedback, I edited it.

webknjaz commented 1 year ago

You're integrating various checks but they are not hooks.

OlenaYefymenko commented 1 year ago

You're integrating various checks but they are not hooks.

Yes, only part of them refers to "hooks". Therefore, I assume that the title of PR should be "Integrate pre-commit into project" (pre-commit - broader concept). In description, I will write: "Settings the pre-commit will enable files inspection before committing. The check includes the following: ...

webknjaz commented 1 year ago

I'd suggest "pre-commit tool" since just "pre-commit" is ambiguous and thus could be misleading.

webknjaz commented 1 year ago

"hooks" is an internal concept of the pre-commit tool. But when used without clarification, one might think that the term is referring to Git when it does not.

OlenaYefymenko commented 1 year ago

But when used without clarification, one might think that the term is referring to Git when it does not.

If I understood you correctly. When used “hooks” without clarification, one might think about this .git/hooks

webknjaz commented 1 year ago

Yep

OlenaYefymenko commented 1 year ago

All the necessary changes were made. Merged to main

webknjaz commented 1 year ago

All the necessary changes were made. Merged to main

No, you didn't. You just closed the PR and re-opened it back again... You'll be able to merge once the in-diff conversations are marked as resolved.

OlenaYefymenko commented 1 year ago

No, you didn't. You just closed the PR and re-opened it back again... You'll be able to merge once the in-diff conversations are marked as resolved.

I understand it. Yes, it was a misstep