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.

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

[Internal] Remove unused decorator dependency #651

Merged
merged 1 commit into from
Apr 26, 2021
Merged

[Internal] Remove unused decorator dependency #651

merged 1 commit into from
Apr 26, 2021

Conversation

TheKevJames
Copy link
Contributor

What does this PR do?

This PR removes the unused dependency on decorator, which as far as I can tell is completely unused. It appears this library was introduced as a dependency in the first commit to this repo but never actually used directly.

This will help make installations faster and slimmer as well as reducing some maintenance (eg. see #646, where the datadogpy build was broken due to environment mismatches in this dependency).

Description of the Change

Removes the unused dependency from the setup.py file. This will prevent it from being included at installation time of datadogpy.

Alternate Designs

N/A

Possible Drawbacks

If the decorator library is being used in some non-standard way, this could remove something we're relying on (note: see "Verification Process" for why I don't think this is the case).

Verification Process

I verified this change through:

  • manual audit of datadogpy: no instances of accessing the decorator library were found
  • running the tox unit and admin-only tests on all compatible cpython and pypy versions: all passed
  • auditing the decorator codebase for anything "unusual" such as install-time side-effects or other such things we might be "magically" relying on: did not find any

Additional Notes

N/A

Release Notes

N/A

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

Either this is unused and we can avoid it as unnecessary or it's
somehow used by magic and I'm about to learn something.
@TheKevJames TheKevJames requested a review from a team as a code owner April 24, 2021 20:17
@TheKevJames
Copy link
Contributor Author

The contributing instructions tell me to add a changelog label (probably changelog/no-changelog in this case?) but I do not seem to have permissions to do so. Is there a process I am unaware of here?

@therve therve added the changelog/Fixed Fixed features results into a bug fix version bump label Apr 26, 2021
@therve
Copy link
Contributor

therve commented Apr 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@therve
Copy link
Contributor

therve commented Apr 26, 2021

Thanks for your patch!

@therve therve merged commit ec571f6 into DataDog:master Apr 26, 2021
@TheKevJames TheKevJames deleted the nuke-decorator branch April 26, 2021 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Fixed Fixed features results into a bug fix version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants