add ci script to ckeck for git unresolved conflicts#23961
add ci script to ckeck for git unresolved conflicts#23961cedric-anne merged 1 commit intoglpi-project:11.0/bugfixesfrom
Conversation
stonebuzz
left a comment
stonebuzz
left a comment
There was a problem hiding this comment.
Could this be added directly within a Rector rule (perhaps), so that we can also benefit from it across plugins as well?
|
Rector is not the appropriate tool, but the idea good. |
| if [[ -n $(grep "warning" extract.log) ]]; then exit 1; fi | ||
|
|
||
| echo "Check for unresolved git merge conflict markers..." | ||
| CONFLICT_MATCHES=$(grep -rn '<<<<<<<\|>>>>>>>' \ |
There was a problem hiding this comment.
======= should be detected too.
| CONFLICT_MATCHES=$(grep -rn '<<<<<<<\|>>>>>>>' \ | |
| CONFLICT_MATCHES=$(grep -rn '<<<<<<<\|=======\|>>>>>>>' \ |
There was a problem hiding this comment.
We have suite of = signs in the code:
install/empty_data.php: 'content_text' => '======================================================================
install/empty_data.php:======================================================================',
tests/imap/MailCollectorTest.php:================================
Those should be changed to apply suggestion.
There was a problem hiding this comment.
does this === have a special meaning in glpi_notificationtemplatetranslations table ? @stonebuzz @trasher ? or can I just remove them ? (a quick search in code tells me there is nothing special with this =)
There was a problem hiding this comment.
Theoretically, the chevrons alone are enough, but if you want, I'll add this @cedric-anne
There was a problem hiding this comment.
Manual conflict resolution can result in having only the ======= line remaining, so if we want a complete detection, it is better to add it.
There was a problem hiding this comment.
Yep, I agree it should be detected.
There was a problem hiding this comment.
does this
===have a special meaning inglpi_notificationtemplatetranslationstable ?
Seems like it's just a kind of "visual separator"; I guess it can be replaced.
There was a problem hiding this comment.
does this === have a special meaning in glpi_notificationtemplatetranslations table ? @stonebuzz @trasher ? or can I just remove them ? (a quick search in code tells me there is nothing special with this =)
It is part of the template text, isn't it? Not saying this template doesn't deserve to be changed, but it is completely unrelated to the scope of this PR.
My opinion is that the goal of this PR is to spot forgotten conflicts, which it does very well.
Spending time handling all possible cases where someone miss-edited a conflict is out of scope and probably not worth it (there are a lot more possibilities than just some "===" left over).
There was a problem hiding this comment.
Almost everytime I have a conflict to handle, I do it manually. So there are the same risk to forgot a ======== line than to forgot a <<<<<<<< line.
Anyway, this can be done later.
5 participants
Description
Add a ci script to check for unresolved git conflicts.
To avoid pr like #23834 to be merged.