Minor changes to Zone11 msgids#1348
Merged
tgreenx merged 3 commits intoJun 12, 2024
Merged
Conversation
Do not refer to “SPF version 1” but just “SPF” because the former is clunky and, until there is a new incompatible version of Sender Policy Framework around, unnecessary. Bring the msgids a bit closer in line to the specification.
tgreenx
previously approved these changes
May 21, 2024
Contributor
There was a problem hiding this comment.
LGTM. I suggest to wait for zonemaster/zonemaster#1264 to be completed first before merging.
matsduf
reviewed
May 21, 2024
| Z11_SPF1_SYNTAX_ERROR => sub { | ||
| __x # ZONE:Z11_SPF1_SYNTAX_ERROR | ||
| 'The SPF version 1 policy has a syntax error. Policy retrieved from the following nameservers: {ns_ip_list}.', @_; | ||
| 'The zone’s SPF policy has a syntax error. Policy retrieved from the following nameservers: {ns_ip_list}.', @_; |
Contributor
There was a problem hiding this comment.
To be strict, the policy is a feature of a name (mail domain) not a zone. In a zone you could have multiple SPF policies for different mail domains, can't you. E.g. in the zone test.xa you could have one for afnic.test.xa and another for iis.test.xa given that those are used for mail addresses, e.g. info@afnic.test.xa and info@iis.test.xa, respectively. Zonemaster only tests the SPF policy of the mail domain equal to the zone apex, but that is still just a name. What do you say about:
The SPF policy of the zone apex has a syntax error. Policy retrieved from the following nameservers: {ns_ip_list}.
Contributor
Author
There was a problem hiding this comment.
Excellent suggestion. I’ll amend the msgid on both sides.
matsduf
reviewed
May 22, 2024
matsduf
previously approved these changes
May 22, 2024
31c9fee to
36db730
Compare
matsduf
previously approved these changes
May 24, 2024
tgreenx
requested changes
Jun 11, 2024
Contributor
|
@marc-vanderwal I just merged zonemaster/zonemaster#1264. If you approve of my aforementioned changes in this PR, then you can merge it. |
Technically, any domain, not just zones, can have SPF policies.
Zonemaster will only fetch the domain under test’s SPF policy. This
means that language referring to “the zone’s SPF policy” is technically
incorrect.
This commit updates the following messages tags to remove such language:
* Z11_INCONSISTENT_SPF_POLICIES;
* Z11_NO_SPF_FOUND;
* Z11_SPF1_SYNTAX_ERROR;
* Z11_SPF1_SYNTAX_OK.
The three last tags in the list get a new argument, {domain}, which is
necessary for the msgid to stay meaningful and user-friendly. The
alternative would be to say something like “the SPF policy at the zone’s
apex”, but “the SPF policy at domain.example” is much better. This is,
however, a change that should ideally be addressed by a database
migration script so that old tests stay meaningful.
36db730 to
90cbcb7
Compare
According to Locale::TextDomain, __x() is only meant for translatable strings with arguments in them. If those strings do not have any arguments, calling __() is sufficient.
tgreenx
approved these changes
Jun 12, 2024
Member
Release testing for v2024.1I've verified that the new text is reported for a couple of the updated messages. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR alters the msgids for Zone11 slightly.
Context
See #1328.
Changes
Small changes to the messages for Zone11.
How to test this PR
Compare the new messages with zonemaster/zonemaster#1264.