search

OCA / hr-attendance

HR Attendance OCA modules for Odoo
GNU Affero General Public License v3.0
50 stars 119 forks source link

[18.0][MIG] hr_attendance_reason #193

Open TomTietze opened 1 week ago

victoralmau commented 6 days ago

Please, cherry-pick https://github.com/OCA/hr-attendance/pull/192 to commit history.

TomTietze commented 6 days ago

@victoralmau Thanks for your feedback! I now added the changes from PR #192 and I squashed all my commits into one.

victoralmau commented 6 days ago

According to the guidelines it is necessary to add all the commit history, you can follow the process indicated in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0.

TomTietze commented 6 days ago

@victoralmau I'm not sure what you mean. You said I should merge my commits into one. I have done that, so there is no further commit history. Or am I not understanding this correctly?

victoralmau commented 6 days ago

According to https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0 in the migration there should be all commit history + pre-commit auto fixes + Migration to 18.0 commit (only one). My initial comment was that the changes after the migration commit should be in 1 commit (Migration to 18.0).

TomTietze commented 6 days ago

@victoralmau Thank you! Now I found out how it worked. I have add the commit history from 17.0 branch for the module hr_attendance_reason

TomTietze commented 5 days ago

@victoralmau Thanks for you review and feedback! I made my comments to your points.

To the extra comments:

  1. I will add the missing commit. I think I forgot to update the 17.0 branch in my fork after the newest change.
  2. I made the changes in translations because for some languages they are not complete and not align with the current Odoo version (es.do -> Odoo 11, de.po -> Odoo 15, bg.po -> Odoo 14, fr.po -> Odoo 14, it.po -> Odoo 15). As this is my first PR for the OCA, I didn't know anything about weblate yet, but I will have a look.
victoralmau commented 5 days ago

Ping @pedrobaeza

pedrobaeza commented 5 days ago

The changes seem reasonable.

TomTietze commented 2 days ago

@victoralmau

  1. I update the branch again to add the missing commit from branch 17.0.
  2. For the translations I have registered for Weblate and I'm now waiting for access. But how can I do the translations for my development there before this PR is merged? That is not possible right?
pedrobaeza commented 2 days ago

@TomTietze it's possible to add the translations in the migration PR, but do it in a separate commit with the tag [I18N]. Weblate is the recommended method for the rest of the cases.

TomTietze commented 2 days ago

@pedrobaeza Thanks for the input! I split the changes in translations into a separate commit with the [I18N] tag.