Skip to content
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

[12.0][ADD] add fleet_vehicle_fuel_type_ethanol #54

Merged

Conversation

marcelsavegnago
Copy link
Member

No description provided.

@marcelsavegnago marcelsavegnago force-pushed the 12.0-add-fleet_vehicle_additional_fuel_type branch from 53f2d5e to be0c8a1 Compare April 1, 2021 05:08
Copy link

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'd rename this module as fleet_vehicle_fuel_type_ethanol
Both added fuel types are related to it.

I tried to find the reason why Odoo made this a Selection field rather than a One2many. All I could find is this: https://github.com/odoo/odoo/blob/13004de8adf9578e775017ada85e13c3a2f966e7/addons/l10n_be_hr_payroll_fleet/models/fleet.py#L49

In any case, I don't see any conflicts with this module.. unless you use belgium localization and you want to compute co2 for ethanol cars

#. module: fleet_vehicle_additional_fuel_type
#: selection:fleet.vehicle,fuel_type:0
msgid "Ethanol"
msgstr "Ethanol"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Translate Ethanol to Etanol.
msgstr "Etanol"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the translation files because the .pot itself is in gitignore and I ended up forcing it to be included in git. For now, I will wait for the return of @ivan if it is automatically generated and if it is, I will leave it to translate through the weblate.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I just read this!
Yes, they are generated by Travis 😊

@marcelsavegnago
Copy link
Member Author

@ivantodorovich Is the .pot file automatically generated when the module is merged?

@marcelsavegnago marcelsavegnago force-pushed the 12.0-add-fleet_vehicle_additional_fuel_type branch from be0c8a1 to 0a10803 Compare April 1, 2021 15:29
@marcelsavegnago
Copy link
Member Author

Hmm I'd rename this module as fleet_vehicle_fuel_type_ethanol
Both added fuel types are related to it.

done!

I tried to find the reason why Odoo made this a Selection field rather than a One2many. All I could find is this: https://github.com/odoo/odoo/blob/13004de8adf9578e775017ada85e13c3a2f966e7/addons/l10n_be_hr_payroll_fleet/models/fleet.py#L49

In any case, I don't see any conflicts with this module.. unless you use belgium localization and you want to compute co2 for ethanol cars

I understood. I'm going to take a look at this module... thanks

@marcelsavegnago marcelsavegnago changed the title [12.0][ADD] add fleet_vehicle_additional_fuel_type [12.0][ADD] add fleet_vehicle_fuel_type_ethanol Apr 1, 2021
@marcelsavegnago
Copy link
Member Author

ping @mamcode @mymage @rpsjr

Copy link
Member

@mymage mymage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that isn't correct to manage this type of data, that can change, inside the code. It had do be managed with a dedicated model with all the coefficient that can be useful. Anyway, if this is the situation, this is the solution.

@ivantodorovich
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-54-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 2, 2021
Signed-off-by ivantodorovich
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-54-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 2, 2021
Signed-off-by ivantodorovich
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-54-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 2, 2021
Signed-off-by ivantodorovich
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-54-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 2, 2021
Signed-off-by ivantodorovich
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-54-by-ivantodorovich-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit bafe0c2 into OCA:12.0 Apr 2, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at b49dd07. Thanks a lot for contributing to OCA. ❤️

@marcelsavegnago marcelsavegnago deleted the 12.0-add-fleet_vehicle_additional_fuel_type branch April 30, 2021 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants