search

ansible-collections / community.windows

Windows community collection for Ansible
https://galaxy.ansible.com/community/windows
GNU General Public License v3.0
199 stars 153 forks source link

win_dns_record: fix TXT tests and remove unneeded special-case code for TXT records #461

Closed kenyon closed 1 year ago

kenyon commented 1 year ago

Looks like this was forgotten in #201.

kenyon commented 1 year ago

@briantist The very next line in each of those files says GPL v3.0+, so these lines conflict, see my commit message: https://github.com/ansible-collections/community.windows/pull/461/commits/4f7bc5e3cf96ae36e4b27ffbbff4afab04e56b48

The v3.0+ line was there before the SPDX-License-Identifier line, so I figure the line that was there first takes precedence.

jborean93 commented 1 year ago

I would just keep the existing license line for now and separate it out into a different PR. Will have to run it by some more knowledgable people to figure out whether we can actually make that change and which one takes precedence here and I don't want to hold up the test fix itself.

Just an FYI you don't need changelog fragments for test fixes.

kenyon commented 1 year ago

@jborean93 OK, removed the changelog fragment commit and the license commit. Rebased and fixed merge conflict.

jborean93 commented 1 year ago

Looks like you included a commit from another PR in this now as it edits the module and not just the tests.

kenyon commented 1 year ago

@jborean93 commit 678c9b332aa385e81894920adf19a9eacd527c04 was there before and is intentional. Updated PR title.

kenyon commented 1 year ago

Just like in #464 where I removed unneeded special-case code for CNAME records, multiple contributors are not understanding how the code has cleverly handled the need for different powershell command line options for each DNS record type.

kenyon commented 1 year ago

Anything else needed to get this merged?

jborean93 commented 1 year ago

@kenyon apologies for the delay here. Thank you very much for working on these modules and also improving the tests, it is really appreciated. Unfortunately this one conflicts with another of your PRs I just merged. If you can rebase this I'm happy to merge it when it's ready.

kenyon commented 1 year ago

@jborean93 thanks! Rebased, fixed the expected merge conflict, tests passed! 🚀