Skip to content

Update allow list for deps update bot #1480

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

Merged
merged 9 commits into from
Dec 1, 2020
Merged

Conversation

dsherry
Copy link
Contributor

@dsherry dsherry commented Nov 30, 2020

Include woodwork, featuretools etc.

@dsherry dsherry added the enhancement An improvement to an existing feature. label Nov 30, 2020
@dsherry dsherry force-pushed the ds_update_deps_bot_allow_list branch from bf6ce9e to 9a39948 Compare November 30, 2020 23:17
@dsherry dsherry marked this pull request as ready for review November 30, 2020 23:22
@codecov
Copy link

codecov bot commented Nov 30, 2020

Codecov Report

Merging #1480 (37ae681) into main (b815f98) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1480   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         223      223           
  Lines       15025    15025           
=======================================
  Hits        15018    15018           
  Misses          7        7           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b815f98...37ae681. Read the comment docs.

Copy link
Contributor

@angela97lin angela97lin left a comment

Choose a reason for hiding this comment

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

LGTM! 😁 Will be useful to use everything from core-requirements.txt, but hopefully doesn't trigger toooo often?

Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@dsherry This looks great! Does updating the circle-ci check update the github dependency bot? (I don't really know how it works hehe).

I have a question about the grep but it's mainly just for my understanding.

@@ -0,0 +1,3 @@
allow_list=$(cat core-requirements.txt requirements.txt | grep -oE "^[a-zA-Z0-9\-_]+" | paste -d "|" -s -)
Copy link
Contributor

Choose a reason for hiding this comment

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

The grep picks up both scikit-learn and scikit-optimize as just "scikit". Is that ok?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@freddyaboulton yes because we're using grep -E which uses a regex, and "scikit" will match both sklearn and skopt.

@jeremyliweishih
Copy link
Collaborator

@freddyaboulton the updates to the bot are under the changes to .github/workflows/dependency_check.yml. Basically it just outputs the file and the bot detects a change in the branch and puts up a PR.

@freddyaboulton
Copy link
Contributor

@jeremyliweishih Thanks! I get it now 😅

@dsherry dsherry merged commit b213240 into main Dec 1, 2020
@dsherry dsherry mentioned this pull request Dec 1, 2020
@freddyaboulton freddyaboulton deleted the ds_update_deps_bot_allow_list branch May 13, 2022 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement to an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants