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

Migrate Detect Language #19

Closed
abpai94 opened this issue Jul 16, 2024 · 4 comments · Fixed by #20
Closed

Migrate Detect Language #19

abpai94 opened this issue Jul 16, 2024 · 4 comments · Fixed by #20
Labels
enhancement New feature or request
Milestone

Comments

@abpai94
Copy link
Contributor

abpai94 commented Jul 16, 2024

The following classes contain duplicate code:
https://github.com/ltb-project/service-desk/blob/master/lib/detectbrowserlanguage.php
https://github.com/ltb-project/self-service-password/blob/master/lib/detectbrowserlanguage.php
https://github.com/ltb-project/white-pages/blob/master/lib/detectbrowserlanguage.php

These can be migrated to create a new addition to the library to reduce duplication.

@davidcoutadeur
Copy link

I agree that the code could be factorized, but not in ltb-ldap.

The remaining question is: do we create a dedicated project for just one function? Not sure, except if we have other things to factorize.

@coudot
Copy link
Member

coudot commented Jul 16, 2024

We already use LTB LDAP for mail functions, so for me it is ok to add languages functions

abpai94 added a commit to Worteks/ltb-ldap that referenced this issue Jul 17, 2024
abpai94 added a commit to Worteks/ltb-ldap that referenced this issue Jul 17, 2024
@coudot coudot linked a pull request Jul 17, 2024 that will close this issue
@coudot coudot added this to the 0.3.0 milestone Jul 17, 2024
@coudot coudot added the enhancement New feature or request label Jul 17, 2024
@davidcoutadeur
Copy link

We already use LTB LDAP for mail functions, so for me it is ok to add languages functions

Indeed, there are other classes in this library. We should rename the library to reflect the real usage. For example, something like ltb-common

@davidcoutadeur
Copy link

The following classes contain duplicate code: https://github.com/ltb-project/service-desk/blob/master/lib/detectbrowserlanguage.php https://github.com/ltb-project/self-service-password/blob/master/lib/detectbrowserlanguage.php https://github.com/ltb-project/white-pages/blob/master/lib/detectbrowserlanguage.php

These can be migrated to create a new addition to the library to reduce duplication.

Please also open an issue for all projects that need refactoring (if not already done): ssp, service-desk, and white-pages. Just to be sure we don't forget.

abpai94 added a commit to Worteks/ltb-ldap that referenced this issue Jul 17, 2024
abpai94 added a commit to Worteks/ltb-ldap that referenced this issue Jul 18, 2024
abpai94 added a commit to Worteks/ltb-ldap that referenced this issue Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants