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

Consistent spelling of "Lua record" in logs #15180

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

miodvallat
Copy link
Contributor

Short description

While trying to help a user trying to make use of Lua records, I couldn't help but notice the related log messages would use different spellings for "Lua".

This PR attempts to homogeneize this, to make searching in the logs easier.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@coveralls
Copy link

coveralls commented Feb 20, 2025

Pull Request Test Coverage Report for Build 13431348177

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 5 of 15 (33.33%) changed or added relevant lines in 3 files are covered.
  • 43 unchanged lines in 12 files lost coverage.
  • Overall coverage decreased (-0.01%) to 64.506%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pdns/packethandler.cc 0 2 0.0%
pdns/lua-record.cc 0 8 0.0%
Files with Coverage Reduction New Missed Lines %
modules/gpgsqlbackend/gpgsqlbackend.cc 1 88.62%
pdns/packethandler.cc 1 73.09%
pdns/validate.cc 1 68.38%
pdns/backends/gsql/gsqlbackend.hh 2 97.71%
pdns/recursordist/sortlist.cc 2 72.94%
pdns/iputils.hh 3 78.84%
pdns/recursordist/recpacketcache.hh 3 89.55%
pdns/recursordist/test-syncres_cc2.cc 3 88.85%
pdns/rcpgenerator.cc 5 90.1%
pdns/recursordist/test-syncres_cc1.cc 5 89.91%
Totals Coverage Status
Change from base Build 13429368953: -0.01%
Covered Lines: 127644
Relevant Lines: 166895

💛 - Coveralls

@Habbie
Copy link
Member

Habbie commented Feb 20, 2025

The docs and the database call it LUA, so I feel that's also the casing we should consistently use in logs, even if it looks a bit shouty

@miodvallat
Copy link
Contributor Author

The doc say the correct spelling is "Lua" but due to DNS constraints the records need to be called "LUA". Since logs are not DNS records, I thought "Lua" would be a better choice.

But I have no problem changing the logs to "LUA" as long as they're consistent, if you prefer this spelling.

@Habbie
Copy link
Member

Habbie commented Feb 20, 2025

I should have looked at the docs before I made that claim! I do prefer 'Lua' but I'm unsure which would be less confusing.

Ok, let's go with 'Lua' :)

Copy link
Member

@Habbie Habbie left a comment

Choose a reason for hiding this comment

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

approved, but I note there are a few mentions of LUA record left in auth-main.cc (settings, not logs) and packethandler.cc (in debug logging)

@miodvallat
Copy link
Contributor Author

I should have looked at the docs before I made that claim! I do prefer 'Lua' but I'm unsure which would be less confusing.

Nobody reads the documentation anyway, so you had the Right™ behaviour. Plus it's hidden in a note in 14.3 Record format.

@miodvallat miodvallat merged commit 0b55a23 into PowerDNS:master Feb 20, 2025
83 checks passed
@miodvallat miodvallat deleted the consistent_logs branch February 20, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants