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.

Sign up for GitHub

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

Jump to bottom

ci: re-enable pre-commit and other non-docs lints for the-epic-split #8161

Merged
merged 2 commits into from
Feb 4, 2024
Merged

ci: re-enable pre-commit and other non-docs lints for the-epic-split #8161

merged 2 commits into from
Feb 4, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Jan 31, 2024 •
edited
Loading

Re-enable disabled tooling for the epic split.

Closes #8152.

@cpcloud cpcloud added ci Continuous Integration issues or PRs developer-tools Tools related to ibis development labels Jan 31, 2024
@cpcloud cpcloud requested review from kszucs, jcrist and gforsyth January 31, 2024 10:43
@cpcloud cpcloud linked an issue Jan 31, 2024 that may be closed by this pull request
ci: re-enable pre-commit for the-epic-split #8152
Closed
@cpcloud cpcloud added this to the 9.0 milestone Jan 31, 2024
@cpcloud cpcloud added the tes-required-for-merge Issues that must addressed before merging the-epic-split branch into main label Jan 31, 2024
@cpcloud
Copy link
Member Author

cpcloud commented Jan 31, 2024 •
edited
Loading

This is blocked by #8005.

@cpcloud cpcloud mentioned this pull request Jan 31, 2024
Merged
@kszucs kszucs force-pushed the the-epic-split branch 2 times, most recently from 3376022 to 432d151 Compare February 1, 2024 17:39
@cpcloud cpcloud force-pushed the tes-pre-commit branch 2 times, most recently from e64c08b to 6fd6a0d Compare February 2, 2024 11:15
@kszucs kszucs force-pushed the the-epic-split branch 2 times, most recently from 432d151 to e4df99b Compare February 2, 2024 11:42
@cpcloud cpcloud force-pushed the tes-pre-commit branch 2 times, most recently from 82c848a to 7555a51 Compare February 3, 2024 13:52
@cpcloud cpcloud added the docs Documentation related issues or PRs label Feb 3, 2024
@cpcloud cpcloud force-pushed the tes-pre-commit branch 3 times, most recently from 2ddb828 to e14cc5a Compare February 3, 2024 16:58
kszucs
kszucs reviewed Feb 3, 2024
View reviewed changes
ibis/expr/types/generic.py
│ True │
└────────────────────────┘
┏━━━━━━━━━━━━━━━┓
┃ InSubquery(a) ┃
Copy link
Member

Choose a reason for hiding this comment

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

This is probably not the nicest name we could generate, in order to change that we can override the name attribute. Just mentioning here, since we should do that for more operations in a follow-up once.

kszucs
kszucs reviewed Feb 3, 2024
View reviewed changes
ibis/expr/types/relations.py Outdated Show resolved Hide resolved
kszucs
kszucs approved these changes Feb 3, 2024
View reviewed changes
Copy link
Member

@kszucs kszucs left a comment

Choose a reason for hiding this comment

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

LGTM

@kszucs kszucs mentioned this pull request Feb 4, 2024
Merged
@kszucs
Copy link
Member

kszucs commented Feb 4, 2024 •
edited
Loading

@cpcloud the table.to_array() method has been removed. It used to return ops.TableArrayView to mark scalar subqueries. This is a separate issue which shouldn't block this PR.

@cpcloud
Copy link
Member Author

cpcloud commented Feb 4, 2024

Refactored CI to allow us to be more granular about what workflows run on a given branch.

@cpcloud
Copy link
Member Author

cpcloud commented Feb 4, 2024

In this case, we're now running the Lint workflow on the-epic-split, but not either of the two docs builds (one pushing to main and the other for PRs)

@cpcloud cpcloud changed the title ci: re-enable pre-commit and docs builds for the-epic-split ci: re-enable pre-commit and other non-docs lints for the-epic-split Feb 4, 2024
@cpcloud cpcloud removed the docs Documentation related issues or PRs label Feb 4, 2024
@cpcloud cpcloud mentioned this pull request Feb 4, 2024
Closed
@cpcloud cpcloud merged commit ffce2ac into ibis-project:the-epic-split Feb 4, 2024
70 of 72 checks passed
@cpcloud cpcloud deleted the tes-pre-commit branch February 4, 2024 14:08
@cpcloud
Copy link
Member Author

cpcloud commented Feb 4, 2024

Whoops, I forgot that CI branch protections aren't enabled on the-epic-split. I will create PRs for any follow-up fixes needed.

This was referenced Feb 4, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kszucs kszucs kszucs approved these changes

@jcrist jcrist Awaiting requested review from jcrist

@gforsyth gforsyth Awaiting requested review from gforsyth

Assignees
No one assigned
Labels
ci Continuous Integration issues or PRs developer-tools Tools related to ibis development tes-required-for-merge Issues that must addressed before merging the-epic-split branch into main
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.

ci: re-enable pre-commit for the-epic-split
2 participants
@cpcloud @kszucs