-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
[IMP] introduce Squash Bot #66
base: main
Are you sure you want to change the base?
Conversation
ec8c127
to
3cbf201
Compare
I like the careful approach to ask the user which commits to rebase or not, however, is there any case where the user would not want to fixup to the previous commit a commit made by any of the SQUASHABLE_AUTHOR_EMAIL? the temp file to do the rebase looks like a quick hack as it's assuming bash will exist in the user environment which probably will in 99% of cases in development environments) + uses subprocess. Nonetheless, I have not found an easy rebase option for libgit in python and it would need to be implemented by its primitive commands. It think it will need a little bit of testing (not done yet from my side) |
try running oca-port on this one: Then try squashing the rest of the commits. It loops on the same commit but does nothing. If you cannot reproduce this issue let me know (as it will be environment related stuff and I can help) Logs:
|
3cbf201
to
83cc505
Compare
Hello @giarve, I provided a fix Why it happened
Solution
I tested it on hr_payroll_period, and it worked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice feature, still didn't test it but I will, thank you!
TRANSLATION_SUMMARY = [ | ||
"Added translation using Weblate", | ||
"Translated using Weblate", | ||
] | ||
SQUASHABLE_SUMMARY = [ | ||
"Update translation files", | ||
] | ||
SQUASHABLE_AUTHOR_EMAIL = [ | ||
"transbot@odoo-community.org", | ||
"noreply@weblate.org", | ||
"oca-git-bot@odoo-community.org", | ||
"oca+oca-travis@odoo-community.org", | ||
"oca-ci@odoo-community.org", | ||
"shopinvader-git-bot@shopinvader.com", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These values are also declared in port_addon_pr.py
module.
I would rename SQUASHABLE_AUTHOR_EMAIL
to AUTHOR_EMAILS
, then use them in your code below and in port_addon_pr.py
with g.AUTHOR_EMAILS
and g.TRANSLATION_SUMMARIES
in replacement of AUTHOR_EMAILS_TO_SKIP
and SUMMARY_TERMS_TO_SKIP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it (quickly), some feedback:
- like @giarve said, asking the user if he wants to squash each commit doesn't help the workflow (I tested on a module with 30+ translations commits), so the added value compared to a
git rebase
after the migration done byoca-port
is still far easier (but as you said, nobody is doing it!) - the bot should detect the list of commits to squash, and propose to the user to squash all of them in a go
Thank you for this. However I have been thinking about another complementary approach also. If you look in OCA/l10n-brazil for instance you will find out that may be up to 20% of the commits are weblate commits generating a lot of pollution in the commit history! What about using a tool like git filter-branch or git filter-repo to filter the i18n module directory entirely and only do one commit with the latest i18n version? This is also equivalent in squashing all commits in i18n even when they are not consequitive. We are generally not interested in preserving all weblate iterations from the previous module versions... |
I think that preserving translator attribution is important. |
Then may be squash the weblate commits together by author? Also this approach can probably be done only during migrations and may be not as effectively during normal ports. |
Context
This change
GIT_SEQUENCE_EDITOR
andGIT_EDITOR
Result
oca_port_squashbot.webm