Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Making checklist optional #5507

Merged
merged 37 commits into from
Feb 9, 2022
Merged

Making checklist optional #5507

merged 37 commits into from
Feb 9, 2022

Conversation

AkshitaB
Copy link
Contributor

@AkshitaB AkshitaB commented Dec 11, 2021 •
edited
Loading

Fixes #5483

@AkshitaB AkshitaB marked this pull request as draft December 11, 2021 00:40
@dirkgr
Copy link
Member

dirkgr commented Jan 12, 2022

@AkshitaB, are you still working on this?

@AkshitaB AkshitaB requested a review from epwalsh February 8, 2022 17:21
@AkshitaB AkshitaB marked this pull request as ready for review February 9, 2022 20:27
epwalsh
epwalsh reviewed Feb 9, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
AkshitaB and others added 2 commits February 10, 2022 03:03
@AkshitaB @epwalsh
Update README.md
85230bb 
Co-authored-by: Pete <petew@allenai.org>
@AkshitaB @epwalsh
Update README.md
c33d7f8 
Co-authored-by: Pete <petew@allenai.org>
epwalsh
epwalsh approved these changes Feb 9, 2022
View reviewed changes
Copy link
Member

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

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

LGTM!

@AkshitaB AkshitaB merged commit 8226e87 into main Feb 9, 2022
@AkshitaB AkshitaB deleted the optional-dep branch February 9, 2022 23:07
@h-vetinari h-vetinari mentioned this pull request Feb 12, 2022
Closed
@h-vetinari
Copy link
Contributor

While refactoring the dependencies from setup.py to requirements.txt (and making checklist optional), two additional requirements were added:

datasets>=1.2.1,<2.0
sqlitedict

I noticed this in conda-forge/allennlp-feedstock#35 - just checking that this was intentional and not a C&P error or similar.

This was referenced Mar 10, 2022
Closed
Merged
@dirkgr
Copy link
Member

dirkgr commented Mar 10, 2022

I think you are right. This must have happened as part of a merge? If the tests for #5593 pass, that's confirmation enough for me that we don't need these.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@epwalsh epwalsh epwalsh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make checklist an optional dependency
4 participants
@AkshitaB @dirkgr @h-vetinari @epwalsh