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.

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

Remove built dependency #847

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Remove built dependency #847

merged 2 commits into from
Nov 3, 2023

Conversation

Jake-Shadle
Copy link
Collaborator

This was pulling in git2 and writing a lot of data that was never used
in the crate. This just changes build.rs to manually read the HEAD
commit information and set an env var.
AFAICT this is actually superior in every way to built since we no
longer have the extra dependencies, and we actually get up to date
information when the commit changes, which AFAICT was not the case with
built since it was not registering file changes.

This was pulling in git2 and writing a lot of data that was never used
in the crate. This just changes build.rs to manually read the HEAD
commit information and set an env var.
AFAICT this is actually superior in every way to built since we no
longer have the extra dependencies, and we actually get up to date
information when the commit changes, which AFAICT was not the case with
built since it was not registering file changes.
@XAMPPRocky
Copy link
Collaborator

These tests weren't flaky until today 😄

@markmandel
Copy link
Contributor

These tests weren't flaky until today 😄

Yeah! It's being really grumpy! I wonder what's up with that.

If you see a flake, please chuck a comment in the PR of a c/p paste of the flake in the PR, and I'll do the same - that makes it easier to search for common flakes through GitHub search and eventually resolve (also we delete old build failures, so it's harder to see the history).

@markmandel markmandel enabled auto-merge (squash) November 3, 2023 17:57
@XAMPPRocky
Copy link
Collaborator

If you see a flake, please chuck a comment in the PR of a c/p paste of the flake in the PR, and I'll do the same - that makes it easier to search for common flakes through GitHub search and eventually resolve (also we delete old build failures, so it's harder to see the history).

It wasn't even the same test though which is odd, it was different tests.

@markmandel markmandel merged commit 98943b0 into main Nov 3, 2023
5 checks passed
@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: 2dbf39bb-babb-4613-8999-3c13c40a464b

The following development images have been built, and will exist for the next 30 days:

To build this version:

git fetch git@github.com:googleforgames/quilkin.git pull/847/head:pr_847 && git checkout pr_847
cargo build

@markmandel
Copy link
Contributor

It wasn't even the same test though which is odd, it was different tests.

Could be Cloud Build is having a grumpy day.

@markmandel markmandel deleted the remove-built branch November 6, 2023 19:43
@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc area/build-tools Development tooling. labels Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build-tools Development tooling. kind/cleanup Refactoring code, fixing up documentation, etc size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants