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.

Sign up for GitHub

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

Jump to bottom

MAN: Remove duplicate dns options #6309

Closed
wants to merge 1 commit into from
Closed

MAN: Remove duplicate dns options #6309

wants to merge 1 commit into from

Conversation

justin-stephenson
Copy link
Contributor

dns_resolver_server_timeout and dns_resolver_op_timeout are shown twice.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2115171

@justin-stephenson justin-stephenson added Waiting for review Bugzilla no-backport This should go to target branch only. labels Aug 17, 2022
thalman
thalman requested changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@thalman thalman left a comment

Choose a reason for hiding this comment

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

Looks good to me, but since we are touching dns_resolver_server_timeout, could you also fix the indentation in the paragraph about AD CLDAP ping (line 3542)?

@justin-stephenson
Copy link
Contributor Author

Looks good to me, but since we are touching dns_resolver_server_timeout, could you also fix the indentation in the paragraph about AD CLDAP ping (line 3542)?

Done.

thalman
thalman approved these changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@thalman thalman left a comment

Choose a reason for hiding this comment

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

Thank you for the fix, ACK

danlavu
danlavu approved these changes Aug 18, 2022
View reviewed changes
Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

LGTM

@alexey-tikhonov alexey-tikhonov added the Ready to push Ready to push label Sep 16, 2022
@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Sep 16, 2022 •
edited

@justin-stephenson, typically only upstream issues are mentioed in the Resolves: part of a commit message.
Could you please remove reference to RHBZ from the commit message?

@justin-stephenson
MAN: Remove duplicate dns options
dc467c0 
dns_resolver_server_timeout and dns_resolver_op_timeout
are shown twice.
@justin-stephenson
Copy link
Contributor Author

@justin-stephenson, typically only upstream issues are mentioed in the Resolves: part of a commit message. Could you please remove reference to RHBZ from the commit message?

Okay done and also rebased. Was I supposed to create an upstream Issue (associated with the BZ) for this?

Is the 'Resolves" line in commit message used only for generating the upstream release notes?

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Sep 19, 2022 •
edited

Okay done and also rebased.

Thanks.

Was I supposed to create an upstream Issue (associated with the BZ) for this?

Is the 'Resolves" line in commit message used only for generating the upstream release notes?

"Automatically" - yes, are mainly used in release notes (also script automation that pushed patches tries to process mentioned ticket and it is only capable of handling upstream tickets).
So use your best judgment to decide if patch is significant enough to have a dedicated upstream ticket and a release note.

@pbrezina
Copy link
Member

Pushed PR: #6309

  • master
    • a241775 - MAN: Remove duplicate dns options

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Sep 23, 2022
@pbrezina pbrezina closed this Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thalman thalman thalman approved these changes

@danlavu danlavu danlavu approved these changes

Assignees

@thalman thalman

@danlavu danlavu

Labels
Bugzilla no-backport This should go to target branch only. Pushed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@justin-stephenson @alexey-tikhonov @pbrezina @thalman @danlavu @andreboscatto