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

Configure --enable-pdns-option --with-third-party-module #7026

Merged
merged 6 commits into from
Nov 29, 2018
Merged

Configure --enable-pdns-option --with-third-party-module #7026

merged 6 commits into from
Nov 29, 2018

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Oct 5, 2018 •
edited
Loading

Short description

https://github.com/PowerDNS/pdns/wiki/Yak-list

another yak I have with configure is our inconsistent use of --enable-X and --with-X
--enable are our programs's options and --with-X should be external deps

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)

@jsoref jsoref force-pushed the configure-enable-with branch 3 times, most recently from 6d908d2 to b46c839 Compare October 9, 2018 14:48
pieterlexis
pieterlexis approved these changes Oct 26, 2018
View reviewed changes
Copy link
Contributor

@pieterlexis pieterlexis left a comment

Choose a reason for hiding this comment

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

I like it, I'll ask for a second opion in case I missed something

rgacogne
rgacogne approved these changes Nov 2, 2018
View reviewed changes
Copy link
Member

@rgacogne rgacogne left a comment

Choose a reason for hiding this comment

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

While I see nothing technically wrong with this PR (and thanks a lot for the work), every packager is going to hate us for this, especially if we change this on a minor version.

@rgacogne
Copy link
Member

rgacogne commented Nov 2, 2018

PR is conflicted though :-/

@jsoref
Copy link
Contributor Author

jsoref commented Nov 2, 2018

@rgacogne: with some additional effort, we could probably support the old flag names... I don't remember the syntax.

@rubenk
Copy link
Contributor

rubenk commented Nov 28, 2018

I think this is long overdue, thanks. As a downstream packager I don't mind changing some things in spec files.

@Habbie Habbie added this to the auth-4.2.0-alpha1 milestone Nov 29, 2018
@Habbie Habbie merged commit 141e964 into PowerDNS:master Nov 29, 2018
@jsoref jsoref deleted the configure-enable-with branch November 29, 2018 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pieterlexis pieterlexis pieterlexis approved these changes

@rgacogne rgacogne rgacogne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
auth-4.2.0-alpha1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jsoref @rgacogne @rubenk @pieterlexis @Habbie