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] don't check scala code with serverised mill #14787

Merged
merged 1 commit into from
Jan 14, 2025
Merged

[ci] don't check scala code with serverised mill #14787

merged 1 commit into from
Jan 14, 2025

Conversation

ehigham
Copy link
Member

@ehigham ehigham commented Jan 13, 2025 •
edited
Loading

No description provided.

@patrick-schultz Graphite App
Copy link
Collaborator

Calling back out to check-hail-fast is a bit odd. Was there something preventing moving that into hail/Makefile too?

@ehigham Graphite App
Copy link
Member Author

ehigham commented Jan 13, 2025

Yeah agree but that logic is shared by all other top-level modules so I didn't think duplicating that was the right call.

@ehigham Graphite App
Copy link
Member Author

ehigham commented Jan 13, 2025

An alternative approach to the one that was taken is to write generic makerules and then include those rules in each Makefile. That was more work than I was prepared to do for this change.

patrick-schultz
patrick-schultz approved these changes Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

Thanks Ed! Hopefully this fixes the weird failures I've been seeing in CI.

@ehigham Graphite App
Copy link
Member Author

ehigham commented Jan 13, 2025 

version.mk:3: *** "git rev-parse HEAD" failed to produce output.  Stop.
make[1]: Leaving directory '/io/repo/hail'
make: *** [Makefile:51: check-hail] Error 2

Ahhh that's why mill was invoked from the top-level Makefile. bugger.

@ehigham ehigham force-pushed the ehigham/make-check-hail branch from 5704120 to b4ab11e Compare January 14, 2025 14:16
@ehigham ehigham changed the title [ci] check-hail without mill server [query] mill-ify hail version Jan 14, 2025
@ehigham ehigham force-pushed the ehigham/make-check-hail branch from b4ab11e to 57e81e2 Compare January 14, 2025 15:48
@ehigham ehigham force-pushed the ehigham/make-check-hail branch from 192365f to a37c08d Compare January 14, 2025 17:59
@ehigham ehigham changed the title [query] mill-ify hail version [ci] don't check scala code with serverised mill Jan 14, 2025
@ehigham ehigham requested review from grohli and removed request for cjllanwarne January 14, 2025 21:36
@hail-ci-robot hail-ci-robot merged commit 603ebd9 into hail-is:main Jan 14, 2025
3 checks passed
@ehigham ehigham deleted the ehigham/make-check-hail branch January 17, 2025 20:27
grohli pushed a commit to grohli/hail that referenced this pull request Jan 27, 2025
@ehigham @grohli
[ci] don't check scala code with serverised mill (hail-is#14787)
36e5b06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grohli grohli grohli approved these changes

@patrick-schultz patrick-schultz patrick-schultz 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.
4 participants
@ehigham @patrick-schultz @grohli @hail-ci-robot