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

Fix issue 319 #326

Merged
merged 2 commits into from Jan 3, 2018

Conversation

Projects
None yet
4 participants
@mtoma
Contributor

mtoma commented Dec 14, 2017

No description provided.

@mtoma mtoma requested review from mattias-p and matsduf Dec 14, 2017

@mattias-p mattias-p referenced this pull request Dec 15, 2017

Merged

Fix issue 292 #324

@mattias-p

Looks good except for indentation.

@@ -58,7 +58,7 @@ sub run {
}
$domain = $self->to_idn( $domain );
if (defined $params->{ipv4} || defined $params->{ipv4}) {

This comment has been minimized.

@mattias-p

mattias-p Dec 15, 2017

Contributor

Could you update this file to be consistent about spaces and tabs in the indentation?

@mattias-p

mattias-p Dec 15, 2017

Contributor

Could you update this file to be consistent about spaces and tabs in the indentation?

@sandoche2k sandoche2k added this to the 2017.4 milestone Dec 19, 2017

@sandoche2k

This comment has been minimized.

Show comment
Hide comment
@sandoche2k

sandoche2k Dec 21, 2017

Contributor

@mtoma please merge after resolving the conflicts

Contributor

sandoche2k commented Dec 21, 2017

@mtoma please merge after resolving the conflicts

@mattias-p

This comment has been minimized.

Show comment
Hide comment
@mattias-p

mattias-p Dec 21, 2017

Contributor

When resolving the conflict, please also exclude my merge commit from this PR. It doesn't belong here.

Contributor

mattias-p commented Dec 21, 2017

When resolving the conflict, please also exclude my merge commit from this PR. It doesn't belong here.

@mtoma mtoma dismissed stale reviews from mattias-p and matsduf via 4e519b6 Dec 21, 2017

@matsduf

Please rebase your branch fix-issue-319 on updated version of branch develop so that only relevant changes are referenced.

@mattias-p

Looking at the list of commits, the commit messages mention all sorts of issue numbers. Do all those commits really belong in this PR? If the branch has attracted commits that don't belong here, please remove them with git rebase -i and git force push to clean up the PR.

I want to be sure I'm reviewing the right changes, and also I'd like to minimize future confusion in case we ever have reason to take a closer look at the history of one of the lines changed in this PR.

@sandoche2k

This comment has been minimized.

Show comment
Hide comment
@sandoche2k

sandoche2k Jan 3, 2018

Contributor

@matsduf @mattias-p I see only two commits with respect to #319 and indentation.

Contributor

sandoche2k commented Jan 3, 2018

@matsduf @mattias-p I see only two commits with respect to #319 and indentation.

@matsduf

matsduf approved these changes Jan 3, 2018

@mtoma mtoma merged commit 3f09845 into zonemaster:develop Jan 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment