varnishcache-friends / varnish3to4

Varnish 3 to 4 migration script
BSD 2-Clause "Simplified" License
125 stars 25 forks source link

Comments should not be touched #3

Closed huayra closed 7 years ago

huayra commented 7 years ago

According to the VCL.BNF definition https://www.varnish-cache.org/trac/wiki/VCL.BNF, VCL has certain ways to add comments:

comment ::= / !(/|/) / // !(\n) $

!(\n)* $

But when trying to convert files from 3 to 4 that include subroutine names (say vcl_fetch) it will blindly be translated within the comment (say into vcl_backend_response) and that should probably be avoided.

Is this something you would consider adding to the script?

fgsch commented 7 years ago

This is on purpose.

In which cases you would want to update the name of the subroutines but keep the old names in the comments?

huayra commented 7 years ago

The explanation on a comment is made for the 3.x context. If anything we could re-write a 4.x version of the comment right below, so that you have ALL the context.

We met this problem a couple of times when using the script and it was hard to figure out the issue.

I.e. the result would be something like:

# comment on the vcl_fetch subroutine
# varnish3to4 # comment below. See above line for original comment
# comment on the vcl_backend_response subroutine
# varnish3to4 # end of comment change

Or something like it

fgsch commented 7 years ago

What if we document it? Either we replace it or we don't, I don't see the point on adding a new comment and keeping the old one tbh.

huayra commented 7 years ago

Documenting it should be enough. PR: https://github.com/fgsch/varnish3to4/pull/8