Refactor shared hooks settings #125
Conversation
- local urls do not get cloned nor updated
- due to security reasons and also since local urls for local shared hooks do not make sense
Pull Request Test Coverage Report for Build 806
💛 - Coveralls |
|
@rycus86 |
Fix opensuse by updating git ⚡ - Another `GIT_DIR` in `is_bare_repo` bug with old versions
`githooks.testingTreatFileProtocolAsRemote` to bypass rejection
- Make install legacy transform to change local paths in .shared file
- `--shared` -> `.githooks/.shared` file - `--local` -> local Git config `githooks.shared` - `--global` -> global Git config `githooks.shared`
gabyx
force-pushed
the
feature/refactor-shared-hooks-settings
branch
from
515788c
to
d815089
Compare
Yeah, if it's too hard to pull out those changes, let's merge them, it's just going to be a bit harder to review. Can you merge all you changes into #125 (or #119) instead of creating a new branch on this side? |
|
Jeah I can do that! thanks! I merge into #125 |
…ctor-shared-hooks-settings 💚
|
Lets first dicuss all these changes, then we can discuss the legacy transform, it maybe needs a test (more work ;-)) |
|
@rycus86 : All tests pass. Thanks for the review. |
|
@rycus86 If you find some time reviewing this on the weekend would be great, so I can improve it next week. I wont have time on the weekend, :-) |
|
Sure, let me try to make sure it's reviewed for you by the end of this week. Thanks a lot! |
Co-authored-by: Viktor Adam <rycus86@gmail.com>
|
@rycus86 Please see my latest commits. Also for |
gabyx
force-pushed
the
feature/refactor-shared-hooks-settings
branch
from
d6ad697
to
d857551
Compare
| echo >&2 | ||
| echo "! Info: Because the hash algorithm changed from" >&2 | ||
| echo " $(md5sum) to $(git hash-object)," >&2 | ||
| echo " you unfortunately need to retrust all hooks again." >&2 |
There was a problem hiding this comment.
| echo " you unfortunately need to retrust all hooks again." >&2 | |
| echo " you will need to accept and trust all hooks again" >&2 | |
| echo " if this is not a trusted repository." >&2 |
There was a problem hiding this comment.
Does this code run only once by the way, or once for every repository?
There was a problem hiding this comment.
FYI: This is about: legacy_transform_split_global_shared_entries which runs globally once when detected by legacy_transform_is_ancestor
There was a problem hiding this comment.
Hm, looks like you resolved this without actually changing the text.
It might be good to fix it up afterwards, please.
Co-authored-by: Viktor Adam <rycus86@gmail.com>
- test 119 not working yet, because we need `git worktree list --porcelain` in the update call
In earlier version git `git worktree list` returns a .git directory which we strip off.
|
Coverage went down significantly due to test119, which I dont know how to cover yet. |
I'm still not convinced about the whole commit tracking thing for upgrades, but it doesn't sound like I can talk you out of it. |
|
We could keep the local paths transform out from local commit tracking, and just warn and let it fail after the users try to use them. |
|
Thanks for looking it through again! Maybe you spot some more errors, I hope now all bad ones should be gone. |
Based on discussion in #122. Based on #119.