IBX-7968: Removed obsolete internal doc & added varnish6.vcl

[dabrt]

🎫 Issue	IBX-7968

Description:

Community reports issues when using Varnish 6.6.
We need to cleatly state what version is supported.

• Ask for Code Review.

[vidarl]

varnish5.vcl is for varnish 6.0LTS ( not for varnish 7.1)
varnish7.vcl is for varnish 7.1

So varnish5.vcl should technically be renamed varnish6.vcl, but that might be considered a BC break, so maybe copy it to varnish6.vcl?

[vidarl]

In Ibexa DXP 5.0, varnish5.vcl should be removed, or at least made deprecated

[mnocon]

I agree that we need to clear the confusion around supported versions, but I'm afraid that adding the varnish6 file and maintaining both of them (varnish5, varnish6) at the same time won't work - I feel that that eventually someone forgets to update one of them.

I'm not sure what the best solution is (if we can't rename the file then I'd probably just keep the filenames varnish5 and varnish6 and made it right in v5 🤔 )

[mnocon]

This file should be removed (I recommend adding the .DS_STORE files to the global .gitignore, we can discuss this next week)

[dabrt]

Done

[dabrt]

I agree that we need to clear the confusion around supported versions, but I'm afraid that adding the varnish6 file and maintaining both of them (varnish5, varnish6) at the same time won't work - I feel that that eventually someone forgets to update one of them.

@alongosz We have confronting opinions from Vidar and Marek. What's your say here?

[alongosz]

@dabrt @mnocon @vidarl

I have a suggestion. Let's make a symlink to varnish5.vcl instead (a0016d1). It looks weird on the github diff and I need to check how this behaves on windows, but this is seems what Symfony is doing as well to avoid duplicating recipes.

In a long term I think the place of these files should be in recipes and they should be placed in custom projects. But not sure where exactly and that's for 5.0

@dabrt one more thing, I see you've stated that you're targeting main version. Do you indeed intend to make this change for 5.0 only? main is for 5.0, yet from the context of the discussion I'm not sure if that was your intention

[alongosz]

It looks weird on the github diff and I need to check how this behaves on windows

update: sadly it doesn't work on windows (at least OOTB)... we need to think what to do with it.

[vidarl]

May idea was to do the renaming in 5.0 only at least.. But it might be that we can only deprecate it in 5.0, and to the actuall renaming in v6?

[alongosz]

May idea was to do the renaming in 5.0 only at least.. But it might be that we can only deprecate it in 5.0, and to the actuall renaming in v6?

We should rather rename it in 5.0.

[alongosz]

@dabrt we've discussed this within PHP Team and due to issues with symlinks decided to keep both files as you've originally intended, but I've added an explanation note on the top of each file. The changes have been already applied to the PR.
// FYI @mnocon @vidarl

@dabrt the only issue remains is that these changes now target future 5.0 branch (main) and I'm not sure if this was your intention. If it was then we just can remove varnish5.vcl.

[dabrt]

@dabrt the only issue remains is that these changes now target future 5.0 branch (main) and I'm not sure if this was your intention. If it was then we just can remove varnish5.vcl.

@alongosz Please advise which bundle branch I should clone these changes to, to include it in 4..6 as well.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[Nattfarinn]

Shouldn't it be rebranded by the way? There are few sub ez_* and call ez_*that could be renamed.

[alongosz]

Shouldn't it be rebranded by the way? There are few sub ez_* and call ez_*that could be renamed.

If we can safely do it, we should.
@vidarl are there any BC concerns here? E.g. is it possible for someone to rely on those subroutine names or are they internal to a vcl file?

[vidarl]

If we can safely do it, we should. @vidarl are there any BC concerns here? E.g. is it possible for someone to rely on those subroutine names or are they internal to a vcl file?

Clients can do al kind of changes in the .vlc file, so strictly speaking, any change we do in vcl can potentially be a BC.
But it is fair to call those sub routines that are named ez_* as internal. I would not consider changing their name in 5.0 a problem. But maybe keep them as-is in existing releases

[adamwojs]

@Nattfarinn We will handle rebranding as part of the 5.0 scope

[alongosz]

Thx for the input, Vidar 👍

[alongosz]

Given this is a copy of existing and working VCL, no QA required here. Merging as-is. Thanks everyone for the input.