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

Add Text::CSV_XS as a requirement, remove Text::CSV from Kernel/cpan-lib #2623

Closed
bschmalhofer opened this issue Oct 30, 2023 · 3 comments
Closed
Assignees
Labels
tidying Tidying of the code
Milestone

Comments

@bschmalhofer
Copy link
Contributor

The module Text::CSV is a thin wrapper around Text::CSV_XS with a fallback implementation in Text/CSV_PP.pm. The installation of Text::CSV_XS is proposed in _bin/otobo.CheckModules.pl. The Docker installation uses Text::CSV_XS since forever. Text::CSV_XS is also available in the major Linux distributions. So I think that there is no longer a good reason for keeping a copy in _Kernel/cpan-lib.

Kernel::System::CSV does not have to be adapted.

@bschmalhofer bschmalhofer added the tidying Tidying of the code label Oct 30, 2023
@bschmalhofer bschmalhofer added this to the OTOBO 11.0 milestone Oct 30, 2023
@bschmalhofer bschmalhofer self-assigned this Oct 30, 2023
@bschmalhofer
Copy link
Contributor Author

Text::CSV_XS 1.34 and Text::CSV 1.95 are the versions provided in Ubuntu 18.04. So these versions should be the minimum versions.

bschmalhofer added a commit that referenced this issue Nov 7, 2023
Also, Enhance POD and code comments in Kernel/System/CSV.pm.
Mention that Text::CSV use Text::CSV_XS by default
bschmalhofer added a commit that referenced this issue Nov 7, 2023
after Text::CSV has been removed from Kernel/cpan-lib.
The modules now have to be installed either from packages
provided by the OS, or from CPAN.
Text::CSV_XS is no longer optional.
@bschmalhofer
Copy link
Contributor Author

Discussed this with @svenoe . The decision was to do this.

bschmalhofer added a commit that referenced this issue Nov 7, 2023
@bschmalhofer
Copy link
Contributor Author

The test suite ran successfully. PR is merged. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tidying Tidying of the code
Projects
None yet
Development

No branches or pull requests

1 participant