-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
ci: make wheel builds more robust #8815
Conversation
@@ -22,7 +22,7 @@ $(VENV_SENTINEL): requirements.txt | |||
$(VENV_DIR)/bin/pip install -r requirements.txt | |||
touch $(VENV_SENTINEL) | |||
|
|||
.PHONY: help html doctest linkcheck clean serve md | |||
.PHONY: help html doctest linkcheck clean clean_all serve md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove clean
from here since clean_all
covers that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - doesn't hurt to have it, but agree that it's redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually this is the .PHONY
rule so it's required here
@@ -1,4 +1,4 @@ | |||
-e ../../metadata-ingestion[datahub-rest] | |||
-e ../../metadata-ingestion[datahub-rest,sql-parsing] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DataHubGraph has a lineage method that uses sqlglot, so this helps sphinx not complain
doesn't actually impact much though
Checklist