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

Rework the QType class #14410

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

fredmorcos
Copy link
Contributor

Short description

This adds QType::fromString() which does error checking and throws exceptions when invalid input is provided.

Cleans up the anti-pattern that was previously there where you could only construct a QType object from a string by assigning to a dummy one. Also removes access to internals like chartocode which was being used all around in place of that anti-pattern.

I haven't tried to build dnsdist and the recursor with this change and I'll wait until tomorrow for the CI to give me feedback.

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 Jul 3, 2024

Pull Request Test Coverage Report for Build 9774716334

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

  • 51 of 73 (69.86%) changed or added relevant lines in 13 files are covered.
  • 9059 unchanged lines in 125 files lost coverage.
  • Overall coverage decreased (-12.6%) to 51.92%

Changes Missing Coverage Covered Lines Changed/Added Lines %
modules/lua2backend/lua2api2.hh 0 1 0.0%
modules/pipebackend/pipebackend.cc 0 1 0.0%
modules/remotebackend/remotebackend.cc 1 2 50.0%
pdns/backends/gsql/gsqlbackend.cc 1 2 50.0%
pdns/lua-base4.cc 2 3 66.67%
pdns/qtype.hh 8 10 80.0%
pdns/ws-auth.cc 0 4 0.0%
pdns/qtype.cc 21 32 65.63%
Files with Coverage Reduction New Missed Lines %
pdns/base64.cc 1 80.6%
ext/yahttp/yahttp/cookie.hpp 1 2.7%
pdns/backends/gsql/ssql.hh 1 52.63%
modules/pipebackend/coprocess.hh 1 0.0%
modules/bindbackend/binddnssec.cc 1 56.73%
pdns/pollmplexer.cc 1 83.01%
pdns/comment.hh 1 0.0%
pdns/cachecleaner.hh 2 67.03%
pdns/backends/gsql/gsqlbackend.hh 2 95.43%
pdns/burtle.hh 2 95.71%
Totals Coverage Status
Change from base Build 9762364246: -12.6%
Covered Lines: 34699
Relevant Lines: 59545

💛 - Coveralls

return "TYPE" + std::to_string(qtype);
}

static map<uint16_t, const string> swapElements(const map<const string, uint16_t>& names)

Check notice

Code scanning / CodeQL

Unused static function Note

Static function swapElements is unreachable
This adds QType::fromString() which does error checking and throws exceptions when invalid
input is provided.

Cleans up the anti-pattern that was previously there where you could only construct a
QType object from a string by assigning to a dummy one. Also removes access to internals
like chartocode which was being used all around in place of that anti-pattern.
@coveralls
Copy link

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9776050440

Details

  • 62 of 89 (69.66%) changed or added relevant lines in 21 files are covered.
  • 6461 unchanged lines in 85 files lost coverage.
  • Overall coverage decreased (-3.3%) to 61.16%

Changes Missing Coverage Covered Lines Changed/Added Lines %
modules/lua2backend/lua2api2.hh 0 1 0.0%
modules/pipebackend/pipebackend.cc 0 1 0.0%
modules/remotebackend/remotebackend.cc 1 2 50.0%
pdns/backends/gsql/gsqlbackend.cc 1 2 50.0%
pdns/dnsdistdist/dnsdist-rings.cc 0 1 0.0%
pdns/lua-base4.cc 2 3 66.67%
pdns/recursordist/rec_channel_rec.cc 0 1 0.0%
pdns/recursordist/ws-recursor.cc 0 1 0.0%
pdns/qtype.hh 11 15 73.33%
pdns/ws-auth.cc 0 4 0.0%
Files with Coverage Reduction New Missed Lines %
pdns/base64.cc 1 80.6%
pdns/dnsparser.hh 1 76.83%
pdns/qtype.hh 1 74.07%
pdns/backends/gsql/ssql.hh 1 52.63%
modules/pipebackend/coprocess.hh 1 0.0%
modules/bindbackend/binddnssec.cc 1 56.73%
pdns/comment.hh 1 0.0%
pdns/dnsdistdist/dnsdist-crypto.cc 2 75.72%
pdns/version.cc 2 92.36%
pdns/dnswriter.cc 2 82.07%
Totals Coverage Status
Change from base Build 9775477569: -3.3%
Covered Lines: 118244
Relevant Lines: 162083

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 4, 2024

Pull Request Test Coverage Report for Build 9792228263

Details

  • 64 of 91 (70.33%) changed or added relevant lines in 21 files are covered.
  • 6466 unchanged lines in 84 files lost coverage.
  • Overall coverage decreased (-3.3%) to 61.148%

Changes Missing Coverage Covered Lines Changed/Added Lines %
modules/lua2backend/lua2api2.hh 0 1 0.0%
modules/pipebackend/pipebackend.cc 0 1 0.0%
modules/remotebackend/remotebackend.cc 1 2 50.0%
pdns/backends/gsql/gsqlbackend.cc 1 2 50.0%
pdns/dnsdistdist/dnsdist-rings.cc 0 1 0.0%
pdns/lua-base4.cc 2 3 66.67%
pdns/recursordist/rec_channel_rec.cc 0 1 0.0%
pdns/recursordist/ws-recursor.cc 0 1 0.0%
pdns/qtype.hh 11 15 73.33%
pdns/ws-auth.cc 0 4 0.0%
Files with Coverage Reduction New Missed Lines %
pdns/base64.cc 1 80.6%
pdns/dnsparser.hh 1 76.83%
pdns/qtype.hh 1 74.07%
pdns/backends/gsql/ssql.hh 1 52.63%
modules/pipebackend/coprocess.hh 1 0.0%
modules/bindbackend/binddnssec.cc 1 56.73%
pdns/comment.hh 1 0.0%
pdns/version.cc 2 92.36%
pdns/dnswriter.cc 2 81.91%
ext/yahttp/yahttp/router.cpp 2 56.65%
Totals Coverage Status
Change from base Build 9775477569: -3.3%
Covered Lines: 118257
Relevant Lines: 162070

💛 - Coveralls

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

Successfully merging this pull request may close these issues.

None yet

2 participants