Skip to content

[IMP] controller_report_xls: general improvements and and code cleanup. t#87779 #1701

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

Conversation

CDanielSanSeg
Copy link
Contributor

No description provided.

- Updated __manifest__.py: set license to LGPL-3, switched to HTTPS for
  URL, removed unnecessary keys.
- Removed outdated copyright header from main controller.
- Refreshed es.po with current project metadata.
- Deleted redundant translation files (es_ES, es_MX, es_PA, es_VE) to
  avoid duplication.
- Fix undefined variable 'cssstyle' in get_css_style function
- Fix undefined variable 'result' in match_color_index function
- Improve string comparison using truthiness instead of empty string
  check
@CDanielSanSeg CDanielSanSeg force-pushed the 15.0-general_improvements_to_controller_report_xls branch from d71fbcb to 9cadb79 Compare July 3, 2025 20:13
@CDanielSanSeg CDanielSanSeg changed the title [IMP] controller_report_xls: general improvements to controller report xls. t#87779 [IMP] controller_report_xls: general improvements and and code cleanup. t#87779 Jul 3, 2025
@CDanielSanSeg
Copy link
Contributor Author

@hugho-ad can you check this PR?

I translate the commets of Luis G on PR 1700 to this PR.

Regards.

Copy link
Contributor

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

Is the controller actually used? I see it's not even imported from the main __init__.py file.

@hugho-ad
Copy link
Contributor

hugho-ad commented Jul 3, 2025

@emtz10
puedes crear un dummy PR de villa con estos cambios pls,

para probar que no nos rompa nada en villa

@hugho-ad
Copy link
Contributor

hugho-ad commented Jul 3, 2025

@CDanielSanSeg

tiene razón Luis G, creo que podemos eliminar el dir de controllers, no se esta usando

@luisg123v
a mi no me gustaria tocar esta version, por que implicaria pruebas en varios clientes como villa

prefiero aplicar los cambios en la mig de v16.0

que opinas?

@luisg123v
Copy link
Contributor

@hugho-ad considero que son cambios seguros, desde que no estamos cambiando el comportamiento, sólo corrigiendo lints. En mi opinión, no necesitan pruebas.

- Remove controllers/main.py, as the controller logic has been
  consolidated into reports/report_xls.py.
- Update imports to reflect the new structure.
- Modernize the codebase while maintaining backward compatibility for
  XLS report generation.
@CDanielSanSeg
Copy link
Contributor Author

Hi @hugho-ad, I applied the comments of Luis G. Also, I deleted the controllers folder.

Can you check this MR?

Regards.

Copy link
Contributor

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@luisg123v luisg123v merged commit fe08195 into Vauxoo:15.0 Jul 4, 2025
3 checks passed
@luisg123v luisg123v deleted the 15.0-general_improvements_to_controller_report_xls branch July 4, 2025 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants