Skip to content

Replace // lines with # in whatwg-headers.conf#155

Merged
sideshowbarker merged 1 commit intomainfrom
sideshowbarker/whatwg-headers.conf-fix-comments
May 28, 2021
Merged

Replace // lines with # in whatwg-headers.conf#155
sideshowbarker merged 1 commit intomainfrom
sideshowbarker/whatwg-headers.conf-fix-comments

Conversation

@sideshowbarker
Copy link
Copy Markdown
Member

@sideshowbarker sideshowbarker commented May 27, 2021

I don’t know why this didn’t cause any problems earlier, but since lines starting with // aren’t valid comments in nginx confing files, the lines we had starting with // were causing nginx to fail to parse the config file, and to fail to start.

So, this replaces those // with #.

@sideshowbarker sideshowbarker requested a review from foolip May 27, 2021 13:57
I don’t know why this didn’t cause any problems earlier, but since
lines starting with // aren’t valid comments in nginx config files, the
lines we had starting with // were causing nginx to fail to parse the
config file, and to fail to start.

So, this replaces those // with #.
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/whatwg-headers.conf-fix-comments branch from 999e751 to 9df09ba Compare May 27, 2021 13:59
@annevk
Copy link
Copy Markdown
Member

annevk commented May 27, 2021

Same problem in debian/isindex/nginx/conf/whatwg.conf, right? Looking at 1854325.

@sideshowbarker
Copy link
Copy Markdown
Member Author

Same problem in debian/isindex/nginx/conf/whatwg.conf, right? Looking at 1854325.

Interesting. For some reason those instances don’t cause the server syntax checker to throw, while the ones in the whatwg-headers.conf. My only guess is that it’s due to having an odd vs even number of lines with them. Otherwise I have no clue.

@annevk
Copy link
Copy Markdown
Member

annevk commented May 27, 2021

It seems that per https://nginx.org/en/docs/beginners_guide.html#conf_structure those should be # as well. Not sure what it gets parsed as instead...

Copy link
Copy Markdown
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sideshowbarker can you get this change deployed?

@sideshowbarker sideshowbarker merged commit 9df7688 into main May 28, 2021
@sideshowbarker sideshowbarker deleted the sideshowbarker/whatwg-headers.conf-fix-comments branch May 28, 2021 03:24
@sideshowbarker
Copy link
Copy Markdown
Member Author

@sideshowbarker can you get this change deployed?

Yeah, I had deployed it already from this PR branch. But after merging this, I doublechecked by re-deploying from the main branch, with no failures.

So I think for now at least, as far as the instances of the //-prefixed lines in the other config files, we can just leave those alone for now. But it remains a mystery to me why those don’t cause nginx any problems while the ones in this case did…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants