search

overleaf / toolkit

GNU Affero General Public License v3.0
510 stars 122 forks source link

more specific sharelatex_occurrences in shared-functions.sh #254

Open Pfiffikus opened 1 month ago

Pfiffikus commented 1 month ago

Description

ignoring occurences in e.g. comments seems to be advisable

Contributor Agreement

I confirm I have signed the Contributor License Agreement

ThexXTURBOXx commented 1 month ago

I disagree. Let's take a look at the original variables.env file (before the OVERLEAF upgrade): https://github.com/overleaf/toolkit/blob/1829e7ee2aa2c523a44cb1ea1f9639fcbd53caa1/lib/config-seed/variables.env

Only a single SHARELATEX_* variable is uncommented by default. Suppose that a user migrates this file to the OVERLEAF_* format. His file will be in an inconsistent state: Only a single variable will be properly upgraded, but the rest will not. So, if he decides to enable some of the other variables in the future, he will probably be quite puzzled why they do not modify any behaviour.

IMHO, this PR would do more harm than good.

Pfiffikus commented 1 month ago

I disagree.

What a pity, that you don't pick up the general idea of this PR and rethink the current check strategy ...

Let's take a look at the original variables.env file ...

What about to check in advance of any command the existence of obsolete variables in the running environment?

So, if he decides to enable some of the other variables ...

... or have set them by some other files ...

IMHO, this PR would do more harm than good.

Maybe in detail, but currently there is a need to search for set but obsolete, thus ignored variables in the environment!